[FFmpeg-devel] [PATCH v4 3/6] Pass secondary ogg/opus chained streams metadata.

[Lynne]

On 10/02/2025 20:25, Romain Beauxis wrote:
> These changes parse ogg/opus comment in secondary chained ogg/opus
> streams and attach them as extradata to the corresponding packet.
> 
> They can then be decoded in the opus decoder and attached to the next
> decoded frame.
> 
> libavformat/oggparseopus.c: Parse comments from
>   secondary chained ogg/opus streams and pass them as ogg stream new_metadata.
> 
> libavcodec/opus/dec.c: Unpack comments from secondary chained ogg/opus
> streams and attach them to the next decoded frame.
> ---
>   libavcodec/opus/dec.c      | 25 ++++++++++++++++++++++---
>   libavformat/oggparseopus.c | 16 +++++++++++++++-
>   2 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/opus/dec.c b/libavcodec/opus/dec.c
> index 88a650c81c..cddcefcb5f 100644
> --- a/libavcodec/opus/dec.c
> +++ b/libavcodec/opus/dec.c
> @@ -125,6 +125,8 @@ typedef struct OpusContext {
>       AVFloatDSPContext *fdsp;
>       float   gain;
>   
> +    AVDictionary *pending_metadata;
> +
>       OpusParseContext p;
>   } OpusContext;
>   
> @@ -485,12 +487,24 @@ static int opus_decode_packet(AVCodecContext *avctx, AVFrame *frame,
>       int decoded_samples = INT_MAX;
>       int delayed_samples = 0;
>       int i, ret;
> +    size_t size;
> +    const uint8_t *side_metadata;
>   
> -    if (buf_size > 8 && (
> -       !memcmp(buf, "OpusHead", 8) ||
> -       !memcmp(buf, "OpusTags", 8)))
> +    if (buf_size > 8 && !memcmp(buf, "OpusHead", 8))
>           return buf_size;
>   
> +    if (buf_size > 8 && !memcmp(buf, "OpusTags", 8)) {
> +        /* New metadata */
> +        side_metadata = av_packet_get_side_data(avpkt, AV_PKT_DATA_METADATA_UPDATE, &size);
> +        if (side_metadata) {
> +            av_dict_free(&c->pending_metadata);
> +            ret = av_packet_unpack_dictionary(side_metadata, size, &c->pending_metadata);
> +            if (ret < 0)
> +                return ret;
> +        }
> +        return buf_size;
> +    }

This is definitely not the right way to go about it. You're changing the 
packet layout too, which can potentially break existing users.

What you should do instead is to either create a linearized dictionary 
(e.g. <keylength><key><value_len><value> in a buffer), and pass that as 
extradata, or add a dictionary field to AVPacket, which would then be 
copied over to the frame after decoding, similar to how PTS, duration, 
etc. are carried over from packets.

Having to add custom handling to each codec to handle metadata updates 
is not good design.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xA2FEA5F03F034464.asc
Type: application/pgp-keys
Size: 624 bytes
Desc: OpenPGP public key
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250213/7553d1e5/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250213/7553d1e5/attachment.sig>