[FFmpeg-devel] [PATCHv2 5/5] vorbis: extract metadata from the middle of a stream

[Ben Boeckel]

On Sat, 26 Oct, 2013 at 12:29:26 GMT, wm4 wrote:
> IMO functions for packing/unpacking AVDictionary into side data should
> be factored into separate functions. I believe something related to
> lavfi has to do the same, so it would be better not to duplicate this
> code.
>
> Not sure where it should be put though.

Agreed. avpriv_packet_pack_metadata?

> Also, if metadata becomes empty, no update is performed. Is that as
> intended? (Maybe it's a fringe case.)

Well, if a tag was there before, but isn't updated, there's no
indication that the tag disappeared. Tracking any tag which has come
through here is probably not something worth the trouble.

> Looks like this would trigger a metadata update even for the initial
> metadata? This would be a bit odd, IMO. Though I'm not sure how exactly
> libavformat/utils.c stream detection interacts with this. Maybe someone
> should check.

I could move it back; not a big deal. I was thinking it'd be nice to
have all metadata available in one place instead of two places depending
on whether the stream is new or not.

> Rest appears to be fine, assuming all avpriv_vorbis_parse_frame() were
> replaced.
>
> What about streams that aren't vorbis?

I don't have any streams handy. I guess I could poke MPD to stream in
other codec/containers. They could use the same side_data pattern as
well (I imagine MP3 is probably the one most users would want).

--Ben