On 8/22/2021 4:49 PM, Jesse Obic wrote:
I'm looking at implementing https://trac.ffmpeg.org/ticket/4448
<https://trac.ffmpeg.org/ticket/4448>, and I understand what I need to
do but I'm on the fence about how I should go about doing it.
For the OGG container, cover art is embedded into the file by adding a
METADATA_BLOCK_PICTURE tag in the vorbis comment section. The data of
this tag is a base64 representation of the FLAC embedded image data
structure, which includes the data of the image itself.
I looked at how vorbis comments are currently written, and it's lead me
to `ff_vorbiscomment_write`. It takes in an AVDictionary for metadata,
and an array of AVChapters for chapter writing. This is where I get a
bit concerned.
I see two options as to how I could handle this:
1. Place the METADATA_BLOCK_PICTURE tag in the metadata AVDictionary.
This would be the easiest functionality wise, however it feels like a
bad idea in my gut because the dictionary would be housing a value that
is 1.33 * the size of the image(s) being embedded. So for example, if
you wished to embed a 12 MB image it would require a 16MB malloc which
doesn't feel right.
Would allocating 16M really be a lot, given typically there wouldn't be
many of them?
2. Change ff_vorbiscomment_write to take in an array of AVStream /
AVPacket to write it directly to the output IO stream
As far as I know this would classify as an API/ABI change. However it
would mean that I don't have to allocate such a huge array upfront, and
can encode smaller chunks at a time as I write them to the output stream.
To mitigate it, it could be created as a new function with the
additional parameters, and the old function pointing towards the new one
instead. I'm not entirely sure how APIs / ABIs are exposed and consumed
in C, so I'm not sure if this will fix the issue.
What if.. LOB value reference support was added to AVDictionary, via a
av_dict_set_lob() function and AVDictionaryEntry->lob field. And anytime
av_dict_get() is called without a AV_DICT_NO_RESOLVE flag, it would
simply do a lazy resolve if it lob != NULL and fill in value field (if
not already resolved, of course). This would allow a LOB aware callers
to stream it when it is a LOB (use 'value' when not NULL). It should
even be API/ABI change safe (unless making AVDictionaryEntry bigger
counts as a break).
Anyway.. just a third option.
Oh, and on a side note: av_dict_get() should _probably_ be changed to
return a const AVDictionaryEntry * at some point, since it is internal
and really should never be modified by the caller.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
