[FFmpeg-devel] [PATCH 1/2] MxPEG decoder
Michael Niedermayer
michaelni
Thu Nov 4 01:16:33 CET 2010
On Tue, Nov 02, 2010 at 01:58:21PM +0300, Anatoly Nenashev wrote:
> On 01.11.2010 15:08, Michael Niedermayer wrote:
>> On Mon, Nov 01, 2010 at 02:55:26PM +0300, Anatoly Nenashev wrote:
>>
>>> On 01.11.2010 14:35, Michael Niedermayer wrote:
>>>
>>>> On Mon, Nov 01, 2010 at 02:06:25AM +0300, Anatoly Nenashev wrote:
>>>>
>>>>
>>>>> On 01.11.2010 00:44, Michael Niedermayer wrote:
>>>>>
>>>>>
>>>>>> On Sun, Oct 31, 2010 at 09:20:34PM +0300, Anatoly Nenashev wrote:
>>>>>> [...]
>>>>>>
>>>>>>
>>>>>>
>>>>>>> @@ -195,6 +198,37 @@ int ff_mjpeg_decode_dht(MJpegDecodeContext *s)
>>>>>>> return 0;
>>>>>>> }
>>>>>>>
>>>>>>> +static int mjpeg_allocate_picture(MJpegDecodeContext *s)
>>>>>>> +{
>>>>>>> + if(s->picture.data[0])
>>>>>>> + s->avctx->release_buffer(s->avctx,&s->picture);
>>>>>>> +
>>>>>>> + s->picture.reference= 0;
>>>>>>> + if(s->avctx->get_buffer(s->avctx,&s->picture)< 0){
>>>>>>> + av_log(s->avctx, AV_LOG_ERROR, "mjpeg get_buffer() failed\n");
>>>>>>> + return -1;
>>>>>>> + }
>>>>>>> +
>>>>>>> + return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static int mxpeg_allocate_picture(MJpegDecodeContext *s)
>>>>>>> +{
>>>>>>> + if (s->mxctx.reference_picture.data[0])
>>>>>>> + s->avctx->release_buffer(s->avctx,&s->mxctx.reference_picture);
>>>>>>> + s->mxctx.reference_picture = s->picture;
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>> + s->picture.data[0] = s->picture.data[1] =
>>>>>>> + s->picture.data[2] = s->picture.data[3] = 0;
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> the indention is odd
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> Just a feature of Emacs. Indention removed.
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>> +
>>>>>>> + s->picture.reference= 1;
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>> + if(s->avctx->get_buffer(s->avctx,&s->picture)< 0){
>>>>>>> + av_log(s->avctx, AV_LOG_ERROR, "mxpeg get_buffer() failed\n");
>>>>>>> + return -1;
>>>>>>> + }
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> duplicate code
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> May be not because "mxpeg_allocate_picture" used in two places:
>>>>> ff_mjpeg_decode_sof and mxpeg_decode_mxm.
>>>>>
>>>>>
>>>>>
>>>>>> [...]
>>>>>>
>>>>>>
>>>>>>
>>>>>>> @@ -788,14 +823,29 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah, i
>>>>>>> }
>>>>>>> }
>>>>>>>
>>>>>>> + if (use_mxm_bitmask) {
>>>>>>> + if (s->mb_width*s->mb_height> (s->mxctx.mxm_bitmask_size<< 3)) {
>>>>>>> + av_log(s->avctx, AV_LOG_ERROR, "MXM bitmask is not complete\n");
>>>>>>> + return -1;
>>>>>>> + }
>>>>>>> +
>>>>>>> + init_get_bits(&mxm_bitmask_gb, s->mxctx.mxm_bitmask,
>>>>>>> + s->mxctx.mxm_bitmask_size<< 3);
>>>>>>> +
>>>>>>> + s->picture.pict_type= FF_P_TYPE;
>>>>>>> + s->picture.key_frame= 0;
>>>>>>> + }
>>>>>>> for(mb_y = 0; mb_y< s->mb_height; mb_y++) {
>>>>>>> for(mb_x = 0; mb_x< s->mb_width; mb_x++) {
>>>>>>> + const int copy_mb = use_mxm_bitmask&& !get_bits1(&mxm_bitmask_gb);
>>>>>>> + if (!reference_picture_avail&& copy_mb) continue;
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> cant use_mxm_bitmask be 0 when !reference_picture_avail
>>>>>> (would avoid 1 check per mb)
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> Fixed. Conditions are switched in "if()".
>>>>>
>>>>>
>>>>>
>>>>>> [...]
>>>>>>
>>>>>>
>>>>>>
>>>>>>> @@ -1505,6 +1631,15 @@ not_the_end:
>>>>>>> the_end:
>>>>>>> av_log(avctx, AV_LOG_DEBUG, "mjpeg decode frame unused %td bytes\n", buf_end - buf_ptr);
>>>>>>> // return buf_end - buf_ptr;
>>>>>>> +
>>>>>>> + if (s->avctx->codec_id == CODEC_ID_MXPEG&&
>>>>>>> + !s->mxctx.has_complete_frame) {
>>>>>>> + if (s->picture.key_frame)
>>>>>>> + s->mxctx.has_complete_frame = 1;
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>> + else if (!(s->avctx->flags& CODEC_FLAG_LOW_DELAY))
>>>>>>> + *data_size = 0;
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> mxpeg is always low delay, unless i misunderstand something
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> Not absolutely so. Generally there's no I-Frame concept for MxPEG known
>>>>> from other codecs. Every encoded frame contains some parts of all image.
>>>>> All that we know is after fixed time delay we can collect a full puzzle.
>>>>> Actually some cameras send full JPEG frame in connection and this
>>>>> feature is supported in attached patch but this is not that case on
>>>>> which it is necessary to rely. That's why if CODEC_FLAG_LOW_DELAY is
>>>>> disabled the decoder must collect a full image before return a first
>>>>> decoded frame. Otherwise, if CODEC_FLAG_LOW_DELAY is enabled we may see
>>>>> a frame full of holes at the start of played video.
>>>>>
>>>>>
>>>> i see what you are trying to do but thats misusing that flag
>>>> i think you will have to add a new flag for this, and h264 could use that too
>>>>
>>>>
>>>>
>>> Ok. I will add a new flag. It might be called CODEC_FLAG_COLLECT_FRAME
>>>
>> CODEC_FLAG_RETURN_INCOMPLETE_FRAMES
>> unless someone has a better idea
>>
>>
>>
>
> CODEC_FLAG2_RETURN_INCOMPLETE_FRAMES is defined in new patch
> Makefile | 1
> allcodecs.c | 1
> avcodec.h | 2
> mjpegdec.c | 187 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> mjpegdec.h | 11 +++
> options.c | 1
> 6 files changed, 189 insertions(+), 14 deletions(-)
> 4f468ef9bdc94b7c61d6446cde6cf3c9307b4a13 mxpeg_v4.patch
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 385ae02..7fb41a6 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -259,6 +259,7 @@ OBJS-$(CONFIG_MSMPEG4V3_ENCODER) += msmpeg4.o msmpeg4data.o h263dec.o \
> OBJS-$(CONFIG_MSRLE_DECODER) += msrle.o msrledec.o
> OBJS-$(CONFIG_MSVIDEO1_DECODER) += msvideo1.o
> OBJS-$(CONFIG_MSZH_DECODER) += lcldec.o
> +OBJS-$(CONFIG_MXPEG_DECODER) += mjpegdec.o mjpeg.o
> OBJS-$(CONFIG_NELLYMOSER_DECODER) += nellymoserdec.o nellymoser.o
> OBJS-$(CONFIG_NELLYMOSER_ENCODER) += nellymoserenc.o nellymoser.o
> OBJS-$(CONFIG_NUV_DECODER) += nuv.o rtjpeg.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 89614ab..6104b53 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -150,6 +150,7 @@ void avcodec_register_all(void)
> REGISTER_DECODER (MSRLE, msrle);
> REGISTER_DECODER (MSVIDEO1, msvideo1);
> REGISTER_DECODER (MSZH, mszh);
> + REGISTER_DECODER (MXPEG, mxpeg);
> REGISTER_DECODER (NUV, nuv);
> REGISTER_ENCDEC (PAM, pam);
> REGISTER_ENCDEC (PBM, pbm);
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 705259e..e553c0a 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -249,6 +249,7 @@ enum CodecID {
> CODEC_ID_A64_MULTI,
> CODEC_ID_A64_MULTI5,
> CODEC_ID_R10K,
> + CODEC_ID_MXPEG,
>
> /* various PCM "codecs" */
> CODEC_ID_PCM_S16LE= 0x10000,
> @@ -652,6 +653,7 @@ typedef struct RcOverride{
> #define CODEC_FLAG2_PSY 0x00080000 ///< Use psycho visual optimizations.
> #define CODEC_FLAG2_SSIM 0x00100000 ///< Compute SSIM during encoding, error[] values are undefined.
> #define CODEC_FLAG2_INTRA_REFRESH 0x00200000 ///< Use periodic insertion of intra blocks instead of keyframes.
> +#define CODEC_FLAG2_RETURN_INCOMPLETE_FRAMES 0x00400000 ///< Allow decoder to return incomplete frames.
>
> /* Unsupported options :
> * Syntax Arithmetic coding (SAC)
> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> index e57f74f..37c97ab 100644
> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -112,6 +112,9 @@ av_cold int ff_mjpeg_decode_init(AVCodecContext *avctx)
> if (avctx->codec->id == CODEC_ID_AMV)
> s->flipped = 1;
>
> + if (avctx->codec_id == CODEC_ID_MXPEG)
> + memset(&s->mxctx, 0, sizeof(s->mxctx));
> +
> return 0;
> }
>
> @@ -205,6 +208,37 @@ int ff_mjpeg_decode_dht(MJpegDecodeContext *s)
> return 0;
> }
>
> +static int mjpeg_allocate_picture(MJpegDecodeContext *s)
> +{
> + if(s->picture.data[0])
> + s->avctx->release_buffer(s->avctx, &s->picture);
> +
> + s->picture.reference= 0;
> + if(s->avctx->get_buffer(s->avctx, &s->picture) < 0){
> + av_log(s->avctx, AV_LOG_ERROR, "mjpeg get_buffer() failed\n");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int mxpeg_allocate_picture(MJpegDecodeContext *s)
> +{
> + if (s->mxctx.reference_picture.data[0])
> + s->avctx->release_buffer(s->avctx, &s->mxctx.reference_picture);
> + s->mxctx.reference_picture = s->picture;
> + s->picture.data[0] = s->picture.data[1] =
> + s->picture.data[2] = s->picture.data[3] = 0;
> +
> + s->picture.reference= 1;
> + if(s->avctx->get_buffer(s->avctx, &s->picture) < 0){
> + av_log(s->avctx, AV_LOG_ERROR, "mxpeg get_buffer() failed\n");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
> {
> int len, nb_components, i, width, height, pix_fmt_id;
> @@ -342,14 +376,12 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
> s->avctx->pix_fmt = PIX_FMT_GRAY16;
> }
>
> - if(s->picture.data[0])
> - s->avctx->release_buffer(s->avctx, &s->picture);
> -
> - s->picture.reference= 0;
> - if(s->avctx->get_buffer(s->avctx, &s->picture) < 0){
> - av_log(s->avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> - return -1;
> + if (s->avctx->codec_id == CODEC_ID_MXPEG && !s->got_picture) {
> + if (mxpeg_allocate_picture(s) < 0) return -1;
> + } else {
> + if (mjpeg_allocate_picture(s) < 0) return -1;
> }
looks like s->picture.reference will b wrong for the first pic
[...]
> +static int mxpeg_decode_mxm(MJpegDecodeContext *s, char *buf, int len)
> +{
> + int32_t mb_width, mb_height, bitmask_size, i;
> + uint32_t mb_count;
> +
> + mb_width = AV_RL16(&buf[4]);
> + mb_height = AV_RL16(&buf[6]);
> + mb_count = (uint32_t)mb_width*mb_height;
> +
> + if (!mb_count || mb_count > 0x7FFFFFF8) {
> + av_log(s->avctx, AV_LOG_ERROR, "MXM wrong macroblocks count");
> + return AVERROR(EINVAL);
> + }
> +
> + bitmask_size = (int32_t)(mb_count + 7) >> 3;
> +
> + if (bitmask_size > (len - 12)) {
> + av_log(s->avctx, AV_LOG_ERROR, "MXM bitmask is not complete");
> + return AVERROR(EINVAL);
> + }
> +
> + av_freep(&s->mxctx.comment_buffer);
> + s->mxctx.comment_buffer = buf;
> + s->mxctx.mxm_bitmask = buf + 12;
> +
> + if (!s->got_picture && s->picture.data[0]) {
> + if (mxpeg_allocate_picture(s) < 0) return -1;
> + s->got_picture = 1;
> + }
allocating pictures in the comment parsing code is not a good idea IMHO
also maybe it would be easier to use reget_buffer() ?
[...]
> diff --git a/libavcodec/options.c b/libavcodec/options.c
> index ef7573c..6a8c24d 100644
> --- a/libavcodec/options.c
> +++ b/libavcodec/options.c
> @@ -88,6 +88,7 @@ static const AVOption options[]={
> {"sgop", "strictly enforce gop size", 0, FF_OPT_TYPE_CONST, CODEC_FLAG2_STRICT_GOP, INT_MIN, INT_MAX, V|E, "flags2"},
> {"noout", "skip bitstream encoding", 0, FF_OPT_TYPE_CONST, CODEC_FLAG2_NO_OUTPUT, INT_MIN, INT_MAX, V|E, "flags2"},
> {"local_header", "place global headers at every keyframe instead of in extradata", 0, FF_OPT_TYPE_CONST, CODEC_FLAG2_LOCAL_HEADER, INT_MIN, INT_MAX, V|E, "flags2"},
> +{"return_incomplete_frames", "allow decoder to return incomplete frames", 0, FF_OPT_TYPE_CONST, CODEC_FLAG2_RETURN_INCOMPLETE_FRAMES, INT_MIN, INT_MAX, V|D, "flags2"},
> {"sub_id", NULL, OFFSET(sub_id), FF_OPT_TYPE_INT, DEFAULT, INT_MIN, INT_MAX},
> {"me_method", "set motion estimation method", OFFSET(me_method), FF_OPT_TYPE_INT, ME_EPZS, INT_MIN, INT_MAX, V|E, "me_method"},
> {"zero", "zero motion estimation (fastest)", 0, FF_OPT_TYPE_CONST, ME_ZERO, INT_MIN, INT_MAX, V|E, "me_method" },
that should be a seperate patch
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101104/da5c5e77/attachment.pgp>
More information about the ffmpeg-devel
mailing list