Re: [FFmpeg-devel] [PATCH] avformat/mov.c: Fix to allow custom udta tags through when export_all option specified.

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 3451 bytes --]

On Sun, Jun 29, 2025 at 09:42:27AM +1200, Ken McGaugh wrote:
> On Sun, 29 Jun 2025 at 08:33, Ken McGaugh <ken@mcgaugh.co.uk> wrote:
> 
> > On Sun, 29 Jun 2025 at 7:44 AM, Baptiste Coudurier <
> > baptiste.coudurier-at-gmail.com@ffmpeg.org> wrote:
> >
> >> Hey Ken
> >>
> >> > On Jun 28, 2025, at 1:04 AM, Ken McGaugh <ken@mcgaugh.co.uk> wrote:
> >> >
> >> > On Sat, 28 Jun 2025 at 13:52, Baptiste Coudurier <
> >> > baptiste.coudurier-at-gmail.com@ffmpeg.org <mailto:
> >> baptiste.coudurier-at-gmail.com@ffmpeg.org>> wrote:
> >> >
> >> >> Hi
> >> >>
> >> >>> On Jun 27, 2025, at 5:46 PM, Ken McGaugh <ken@mcgaugh.co.uk> wrote:
> >> >>>
> >> >>> Hi, first time contributing to ffmpeg so I hope I got this right. We
> >> use
> >> >>> custom udta tags in our pipeline but they were not coming through
> >> >>> correctly. Looks like they were incorrectly being routed through the
> >> code
> >> >>> path for "raw" data.
> >> >>> ---
> >> >>> libavformat/mov.c | 2 +-
> >> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>>
> >> >>> diff --git a/libavformat/mov.c b/libavformat/mov.c
> >> >>> index bd6b23466b..1b17f52a16 100644
> >> >>> --- a/libavformat/mov.c
> >> >>> +++ b/libavformat/mov.c
> >> >>> @@ -484,7 +484,7 @@ retry:
> >> >>>                return ret;
> >> >>>            }
> >> >>>        } else return 0;
> >> >>> -    } else if (atom.size > 4 && key && !c->itunes_metadata && !raw) {
> >> >>> +    } else if (atom.size > 4 && !c->itunes_metadata && !raw) {
> >> >>>        str_size = avio_rb16(pb); // string length
> >> >>>        if (str_size > atom.size) {
> >> >>>            raw = 1;
> >> >>> --
> >> >>> 2.39.5 (Apple Git-154)
> >> >>
> >> >> Thanks!
> >> >> Would it be possible for you to share a small sample?
> >> >>
> >> >>
> >> > Certainly. Here is a sample .mov with a handful of custom udta atoms.
> >> For
> >> > example, the '[169]tco' one should have the value "FrameNumber", but
> >> > without this patch it is exported as an empty string.
> >> >
> >> >
> >> https://drive.google.com/file/d/1KJgRueP26cuo1os2OaNqBz2u8-aeUbBS/view?usp=sharing
> >> >
> >>
> >> Thanks!
> >> I feel we should do "if (atom.size > 4 && (key || c->export_all) &&
> >> !c->itunes_metadata && !raw” to be safer.
> >> What do you think ?
> >
> >
> > That would definitely be safer. Thanks for catching that. Do you need me
> > to submit an updated patch?
> >
> 
> Here it is in case you need it from me. Thanks again for your prompt review.
> 
>  ---
>  libavformat/mov.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index bd6b23466b..31ebcebca1 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -484,7 +484,7 @@ retry:
>                  return ret;
>              }
>          } else return 0;
> -    } else if (atom.size > 4 && key && !c->itunes_metadata && !raw) {
> +    } else if (atom.size > 4 && (key || c->export_all) &&
> !c->itunes_metadata && !raw) {
>          str_size = avio_rb16(pb); // string length

theres a linebreak corrupting the patch, check your editors word / line wrap settings
or attach patches

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

For a strong democracy, genuine criticism is necessary, allegations benefit
noone, they just cause unnecessary conflicts. - Narendra Modi

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".