Re: [FFmpeg-devel] [PATCH 2/2] libavformat/mov: fix udta reading in trak box

Re: [FFmpeg-devel] [PATCH 2/2] libavformat/mov: fix udta reading in trak box

[Wang Chuan]

Ping?
On Jan 28, 2022, 11:24 AM +0800, Wang Chuan <ouchuanm@outlook.com>, wrote:
> if we are reading udta in trak box, the data should go to metadata
> of current stream.
>
> Signed-off-by: Wang Chuan <ouchuanm@outlook.com>
> ---
> libavformat/mov.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 1437d160f8..cb983defb3 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -522,7 +522,10 @@ retry:
> str[str_size] = 0;
> }
> c->fc->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
> - av_dict_set(&c->fc->metadata, key, str, 0);
> + if (c->trak_index != -1)
> + av_dict_set(&c->fc->streams[c->trak_index]->metadata, key,
> str, 0);
> + else
> + av_dict_set(&c->fc->metadata, key, str, 0);
> if (*language && strcmp(language, "und")) {
> snprintf(key2, sizeof(key2), "%s-%s", key, language);
> av_dict_set(&c->fc->metadata, key2, str, 0);
> --
> 2.29.2
>
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] libavformat/mov: fix udta reading in trak box

[Jan Ekström]

On Fri, Feb 4, 2022 at 5:24 AM Wang Chuan <ouchuanm@outlook.com> wrote:
>
> Ping?
> On Jan 28, 2022, 11:24 AM +0800, Wang Chuan <ouchuanm@outlook.com>, wrote:
> > if we are reading udta in trak box, the data should go to metadata
> > of current stream.
> >
> > Signed-off-by: Wang Chuan <ouchuanm@outlook.com>
> > ---
> > libavformat/mov.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 1437d160f8..cb983defb3 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -522,7 +522,10 @@ retry:
> > str[str_size] = 0;
> > }
> > c->fc->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
> > - av_dict_set(&c->fc->metadata, key, str, 0);
> > + if (c->trak_index != -1)
> > + av_dict_set(&c->fc->streams[c->trak_index]->metadata, key,
> > str, 0);
> > + else
> > + av_dict_set(&c->fc->metadata, key, str, 0);
> > if (*language && strcmp(language, "und")) {
> > snprintf(key2, sizeof(key2), "%s-%s", key, language);
> > av_dict_set(&c->fc->metadata, key2, str, 0);
> > --
> > 2.29.2

I recall having some patches on my github regarding something related,
will attempt to check this during the week-end.

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

Re: [FFmpeg-devel] [PATCH 2/2] libavformat/mov: fix udta reading in trak box

[Wang Chuan]

Any news？

On 2022/2/4 19:10, Jan Ekström wrote:
> On Fri, Feb 4, 2022 at 5:24 AM Wang Chuan <ouchuanm@outlook.com> wrote:
>> Ping?
>> On Jan 28, 2022, 11:24 AM +0800, Wang Chuan <ouchuanm@outlook.com>, wrote:
>>> if we are reading udta in trak box, the data should go to metadata
>>> of current stream.
>>>
>>> Signed-off-by: Wang Chuan <ouchuanm@outlook.com>
>>> ---
>>> libavformat/mov.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>>> index 1437d160f8..cb983defb3 100644
>>> --- a/libavformat/mov.c
>>> +++ b/libavformat/mov.c
>>> @@ -522,7 +522,10 @@ retry:
>>> str[str_size] = 0;
>>> }
>>> c->fc->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
>>> - av_dict_set(&c->fc->metadata, key, str, 0);
>>> + if (c->trak_index != -1)
>>> + av_dict_set(&c->fc->streams[c->trak_index]->metadata, key,
>>> str, 0);
>>> + else
>>> + av_dict_set(&c->fc->metadata, key, str, 0);
>>> if (*language && strcmp(language, "und")) {
>>> snprintf(key2, sizeof(key2), "%s-%s", key, language);
>>> av_dict_set(&c->fc->metadata, key2, str, 0);
>>> --
>>> 2.29.2
> I recall having some patches on my github regarding something related,
> will attempt to check this during the week-end.
>
> Jan
> _______________________________________________
> 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".

Re: [FFmpeg-devel] [PATCH 2/2] libavformat/mov: fix udta reading in trak box

[Jan Ekström]

On Tue, Feb 8, 2022 at 9:48 AM Wang Chuan <ouchuanm@outlook.com> wrote:
>
> Any news？
>

Sorry, was not able to get to this according to the time line I
expected. Will see if I can find some time for this soon.

The attempt I had done in October was quite similar now that I look at
it again (https://github.com/jeeb/ffmpeg/commits/enable_writing_udta_metadata_for_tracks),
although it seems like I missed c->trak_index , will have to check it
:)

Additionally, when I did the changes a lot of tests had to be updated
as the test would expect the metadata in the main context, as
previously the metadata only got applied globally. The changes in my
commit aren't what's needed as I just committed the changes in test
results to remind myself which tests would require additional changes
or at least review.

Looking at the patchwork side for this patch set, it seems like it
wasn't able to run the tests for you, so you probably did not get any
messages about failing tests?
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=5839

For running tests locally, what I usually do is:

1. configure and build normally
2. `make fate-rsync SAMPLES=../../path/to/fate-suite`
3. `make fate SAMPLES=../../path/to/fate-suite`

this is also documented at https://www.ffmpeg.org/fate.html .

For the movenc patch, I think there was this weird thing where in QTFF
there were two different "name" metadata entries (as well as the
international one you've pointed out). I don't think I mentioned it on
https://github.com/mpv-player/mpv/issues/8488#issuecomment-884381794
as it contains only my initial findings, but I'll have to grep my IRC
logs :D .

Thank you for your contribution and patience,

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

Re: [FFmpeg-devel] [PATCH 2/2] libavformat/mov: fix udta reading in trak box

[Wang Chuan]

On Feb 9, 2022, 7:31 AM +0800, Jan Ekström <jeebjp@gmail.com>, wrote:
> On Tue, Feb 8, 2022 at 9:48 AM Wang Chuan <ouchuanm@outlook.com> wrote:
> >
> > Any news？
> >
>
> Sorry, was not able to get to this according to the time line I
> expected. Will see if I can find some time for this soon.
>
> The attempt I had done in October was quite similar now that I look at
> it again (https://github.com/jeeb/ffmpeg/commits/enable_writing_udta_metadata_for_tracks),
> although it seems like I missed c->trak_index , will have to check it
> :)
>
> Additionally, when I did the changes a lot of tests had to be updated
> as the test would expect the metadata in the main context, as
> previously the metadata only got applied globally. The changes in my
> commit aren't what's needed as I just committed the changes in test
> results to remind myself which tests would require additional changes
> or at least review.
>
> Looking at the patchwork side for this patch set, it seems like it
> wasn't able to run the tests for you, so you probably did not get any
> messages about failing tests?
> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=5839
>
> For running tests locally, what I usually do is:
>
> 1. configure and build normally
> 2. `make fate-rsync SAMPLES=../../path/to/fate-suite`
> 3. `make fate SAMPLES=../../path/to/fate-suite`
>
> this is also documented at https://www.ffmpeg.org/fate.html .
Thanks for your reply!

I have tried running tests on my PC, but I didn’t meet any test failure...
But if I use your patch, the tests failed.

The difference between your patch and my is that you use [s->nb_streams - 1]
but I use [c->trak_index].
It seems like [c->trak_index] is a better choice.
(It seems like if we use [s->nb_streams - 1], the metadata we read will all go to the
last)
Can you check this again?

Thanks
_______________________________________________
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] [PATCH 2/2] libavformat/mov: fix udta reading in trak box

[Wang Chuan]

if we are reading udta in trak box, the data should go to metadata
of current stream.

Signed-off-by: Wang Chuan <ouchuanm@outlook.com>
---
  libavformat/mov.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 1437d160f8..cb983defb3 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -522,7 +522,10 @@ retry:
              str[str_size] = 0;
          }
          c->fc->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
-        av_dict_set(&c->fc->metadata, key, str, 0);
+        if (c->trak_index != -1)
+            av_dict_set(&c->fc->streams[c->trak_index]->metadata, key, 
str, 0);
+        else
+            av_dict_set(&c->fc->metadata, key, str, 0);
          if (*language && strcmp(language, "und")) {
              snprintf(key2, sizeof(key2), "%s-%s", key, language);
              av_dict_set(&c->fc->metadata, key2, str, 0);
-- 
2.29.2

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