Re: [FFmpeg-devel] [PATCH 1/2] avformat/tee: refactor option processing

[Marvin Scholz <epirat07-at-gmail.com@ffmpeg.org>]

On 3 Jul 2025, at 11:08, Nicolas George wrote:

> Marvin Scholz (HE12025-06-25):
>> Would you be fine with just the removal of the messing with
>> the AVDictionary entries then, leaving the macros in place,
>> essentially removing STEAL_OPTION and doing a copy in CONSUME_OPTION?
>>
>> IMHO saving two copies of a string does not justify abusing the
>> AVDictionary API in such a way. This isnt a hot code path either
>> where this would make sense...
>
> It is not just a matter of saving a few cycles. What you propose
> requires writing more code, including error checks and an occasion for
> failure.

If copying two strings fails here, it is highly unlikely any of the following
code, needing much more memory, would have any chance of succeeding.

>
> If it was new code, I would consider it, but changing existing code that
> has been working for years to make it more verbose, more failure prone

Yes it introducers two copies which could fail but the chance for that happening
is so small that I dont think it justifies using hacks like this, which also introduce
risk of turning into more serious issues when the AVDictionary code is changed and someone
is unaware of this hack here.

> and less efficient, no, thanks.

We are talking about copying two option strings here, during setup,
not per frame, not every few seconds.

>
> Let us just document that it is a valid use.
>

I disagree making this hack official.

It will make any further changes to AVDictionary even harder
if we have to consider the user messing with it in such ways.

Let's not sacrifice code quality and proper APIs for the sake of
'efficiency'.

The main reason I want to get rid of this is so that av_dict_get's
return value can be made const which is only blocked by this one
use here. Nowhere else in ffmpegs code do we do this, I really
see no sufficient justification to do it here.

Regards,
Marvin Scholz

> Regards,
>
> -- 
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".