[FFmpeg-devel] [PATCH 2/4] lavd: make lavfi device export the metadata up to the AVFrame.

[Clément Bœsch]

On Thu, Oct 11, 2012 at 11:49:26AM +0200, Stefano Sabatini wrote:
> On date Wednesday 2012-10-10 00:55:11 +0200, Clément Bœsch encoded:
> > TODO: lavc minor bump?
> > ---
> >  libavcodec/avcodec.h | 1 +
> >  libavcodec/utils.c   | 7 +++++++
> >  libavdevice/lavfi.c  | 9 +++++++++
> >  3 files changed, 17 insertions(+)
> > 
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 6c4b630..ca7cd75 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -1008,6 +1008,7 @@ typedef struct AVPacket {
> >  } AVPacket;
> >  #define AV_PKT_FLAG_KEY     0x0001 ///< The packet contains a keyframe
> >  #define AV_PKT_FLAG_CORRUPT 0x0002 ///< The packet content is corrupted
> > +#define AV_PKT_FLAG_META    0x0100 ///< The packet priv points to metadata
> 
> May be acceptable, but I leave this to people with more packet API
> insight.
> 
> Also I'm not sure about the name, "META" (meta packet??) doesn't tell
> much, "METADATA_IN_PRIV" or "PRIV_METADATA" may be more descriptive.
> 

Yep sorry about that; it was a left over from when I was using the
"AV_SIDE_DATA_" prefix (and where it was appropriate to just add "META").
Renamed locally to AV_PKT_FLAG_PRIV_METADATA.

> Also I'd specify something in the docs along the lines:
> Metadata is stored in the demuxer, and is not preserved at the next
> call of av_read_frame().
> 

Right, where would you put that info? As an extension to the define doxy?

> Also missing: API changhes entry for the new symbol.
> 

Added TODO in the commit desc

> >  enum AVSideDataParamChangeFlags {
> >      AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_COUNT  = 0x0001,
> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > index 036cbdf..85b42e5 100644
> > --- a/libavcodec/utils.c
> > +++ b/libavcodec/utils.c
> > @@ -1590,6 +1590,9 @@ int attribute_align_arg avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
> >  
> >          emms_c(); //needed to avoid an emms_c() call before every return;
> >  
> > +        if (avctx->pkt->flags & AV_PKT_FLAG_META)
> > +            picture->metadata = avctx->pkt->priv;
> > +
> >          avctx->pkt = NULL;
> >          if (did_split) {
> >              ff_packet_free_side_data(&tmp);
> > @@ -1713,6 +1716,10 @@ int attribute_align_arg avcodec_decode_audio4(AVCodecContext *avctx,
> >              av_log(avctx, AV_LOG_DEBUG, "skip %d samples due to side data\n",
> >                     avctx->internal->skip_samples);
> >          }
> > +
> > +        if (avctx->pkt->flags & AV_PKT_FLAG_META)
> > +            frame->metadata = avctx->pkt->priv;
> > +
> >          if (avctx->internal->skip_samples) {
> >              if(frame->nb_samples <= avctx->internal->skip_samples){
> >                  *got_frame_ptr = 0;
> > diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
> > index 860c055..6e76b5f 100644
> > --- a/libavdevice/lavfi.c
> > +++ b/libavdevice/lavfi.c
> > @@ -48,6 +48,7 @@ typedef struct {
> >      int *sink_stream_map;
> >      int *sink_eof;
> >      int *stream_sink_map;
> > +    AVDictionary *metadata; ///< metadata of the current packet
> >  } LavfiContext;
> >  
> >  static int *create_all_formats(int n)
> > @@ -76,6 +77,7 @@ av_cold static int lavfi_read_close(AVFormatContext *avctx)
> >      av_freep(&lavfi->sink_eof);
> >      av_freep(&lavfi->stream_sink_map);
> >      av_freep(&lavfi->sinks);
> > +    av_dict_free(&lavfi->metadata);
> >      avfilter_graph_free(&lavfi->graph);
> >  
> >      return 0;
> > @@ -336,6 +338,13 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
> >          memcpy(pkt->data, ref->data[0], size);
> >      }
> >  
> > +    if (ref->metadata) {
> > +        av_dict_free(&lavfi->metadata);
> > +        av_dict_copy(&lavfi->metadata, ref->metadata, 0);
> > +        pkt->flags |= AV_PKT_FLAG_META;
> > +        pkt->priv = lavfi->metadata;
> > +    }
> > +
> >      pkt->stream_index = stream_idx;
> >      pkt->pts = ref->pts;
> >      pkt->pos = ref->pos;
> > -- 
> > 1.7.12.2
> 
> LGTM otherwise.

I'm waiting for more comments about the "abuse" of the priv.

[...]

Thanks for the review :)

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121011/d8a8193d/attachment.asc>