[FFmpeg-devel] MPEG user data handling (read & write sample patches attached)

[Michael Niedermayer]

On Thu, Jul 01, 2010 at 11:41:32AM -0400, Daniel Kristjansson wrote:
> I posted a patch earlier that decoded captions in user data, but
> it used fixed sized arrays and also a suggestion was made that
> the raw user data be passed to the application instead.
> 
> Attached are two patches. One allows writing user data and has been
> tested and works. The other is parses out user data from the video
> stream and passes it out on the video frames but has not been tested.
> 
> I'd like to know if I'm on the right track here before proceeding.
> 
> Anyone writing captions in user data will be generating user data
> anyway, so this avoids having a different interface for reading
> and writing captions. The user_data to cc packet decoder function
> will still be available in MythTV for anyone to look at when working
> on their own app. Also using the same interface for reading and
> writing user data should also make transcoding while preserving
> captions fairly simple to do.


please add a -p when generating patches

anyway, your patches look odd
[...]
> @@ -962,6 +966,17 @@
>                  pic->reference = 3;
>          }
>  
> +        av_freep(&pic->user_data_buf);
> +        pic->user_data_len = 0;
> +        if (s->user_data_len)
> +        {
> +            pic->user_data_buf = av_malloc(s->user_data_len);
> +            memcpy(pic->user_data_buf, s->user_data_buf,
> +                   s->user_data_len);
> +            pic->user_data_len = s->user_data_len;
> +            s->user_data_len = 0;
> +        }
> +

thats odd
why dont you store the user data in the current AVFrame but rather store
it in the main context?


>          pic->coded_picture_number= s->coded_picture_number++;
>  
>          if(ff_alloc_picture(s, pic, 0) < 0)
> Index: libavcodec/mpegvideo.h
> ===================================================================
> --- libavcodec/mpegvideo.h	(revision 23934)
> +++ libavcodec/mpegvideo.h	(working copy)
> @@ -645,6 +645,13 @@
>  
>      DCTELEM (*block)[64]; ///< points to one of the following blocks
>      DCTELEM (*blocks)[8][64]; // for HQ mode we need to keep the best block
> +
> +    /// Used to hold cached user_data about caption packets before the
> +    /// frame for these packets has been created in MPV_frame_start().
> +    uint8_t *user_data_buf;
> +    int user_data_len;
> +    int user_data_alloc;
> +
>      int (*decode_mb)(struct MpegEncContext *s, DCTELEM block[6][64]); // used by some codecs to avoid a switch()
>  #define SLICE_OK         0
>  #define SLICE_ERROR     -1
> Index: libavcodec/mpeg12.c
> ===================================================================
> --- libavcodec/mpeg12.c	(revision 23934)
> +++ libavcodec/mpeg12.c	(working copy)
> @@ -2105,6 +2105,8 @@
>                                    const uint8_t *p, int buf_size)
>  {
>      const uint8_t *buf_end = p+buf_size;
> +    Mpeg1Context   *s1 = avctx->priv_data;
> +    MpegEncContext *s  = &s1->mpeg_enc_ctx;
>  
>      /* we parse the DTG active format information */
>      if (buf_end - p >= 5 &&
> @@ -2121,6 +2123,17 @@
>              avctx->dtg_active_format = p[0] & 0x0f;
>          }
>      }
> +
> +    if (!s->user_data_alloc) {
> +        s->user_data_alloc = (buf_size > 2048) ? buf_size : 2048;
> +        s->user_data_buf = av_malloc(s->user_data_alloc);
> +    } else if (buf_size + s->user_data_len > s->user_data_alloc) {
> +        s->user_data_alloc *= 2;
> +        s->user_data_buf = av_realloc(s->user_data_buf, s->user_data_alloc);
> +    }

av_fast_realloc()

[...]
> Index: libavcodec/mpegvideo_enc.c
> ===================================================================
> --- libavcodec/mpegvideo_enc.c	(revision 23934)
> +++ libavcodec/mpegvideo_enc.c	(working copy)
> @@ -201,6 +201,13 @@
>              }
>          }
>      }
> +
> +    if (src->user_data_size && dst->type == FF_BUFFER_TYPE_INTERNAL)
> +    {
> +        dst->user_data = av_realloc(dst->user_data, src->user_data_size);
> +        dst->user_data_size = src->user_data_size;
> +        memcpy(dst->user_data, src->user_data, src->user_data_size);
> +    }
>  }

with that what has been allocated by the user must be freed by lavc
thats different from other tables like MVs are handled

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100705/408e1dfd/attachment.pgp>