| Message ID | 201612121212.37253.cehoyos@ag.or.at |
|---|---|
| State | New |
| Headers | show |
2016-12-12 12:12 GMT+01:00 Carl Eugen Hoyos <cehoyos@ag.or.at>: > I saw only after writing this patch that a Google employee has sent > a more complicated variant of this patch in November (searching for > a description of the vendor field showed the patch). Ping for the ffmpeg patch. I will add audio vendor to the mov patch. Carl Eugen
On Mon, 12 Dec 2016 12:12:37 +0100 Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: > From 7c26220a8734fe7dc293efe6c13e3baf91defc7e Mon Sep 17 00:00:00 2001 > From: Carl Eugen Hoyos <cehoyos@ag.or.at> > Date: Mon, 12 Dec 2016 12:07:27 +0100 > Subject: [PATCH 2/2] lavf/mov: Export vendor metadata. > > --- > libavformat/mov.c | 5 ++++- > tests/ref/fate/mov-zombie | 2 +- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 0b1c182..a19ebbf 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -1842,6 +1842,7 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, > AVStream *st, MOVStreamContext *sc) > { > uint8_t codec_name[32] = { 0 }; > + uint8_t vendor[5] = { 0 }; > int64_t stsd_start; > unsigned int len; > > @@ -1851,7 +1852,9 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, > > avio_rb16(pb); /* version */ > avio_rb16(pb); /* revision level */ > - avio_rb32(pb); /* vendor */ > + avio_read(pb, vendor, 4); > + if (vendor[0]) > + av_dict_set(&st->metadata, "vendor", vendor, 0); Does this mean transcoding to a format with per-stream tags will add this as a tag?
2017-01-25 14:22 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > On Mon, 12 Dec 2016 12:12:37 +0100 > Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: > >> From 7c26220a8734fe7dc293efe6c13e3baf91defc7e Mon Sep 17 00:00:00 2001 >> From: Carl Eugen Hoyos <cehoyos@ag.or.at> >> Date: Mon, 12 Dec 2016 12:07:27 +0100 >> Subject: [PATCH 2/2] lavf/mov: Export vendor metadata. >> >> --- >> libavformat/mov.c | 5 ++++- >> tests/ref/fate/mov-zombie | 2 +- >> 2 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/libavformat/mov.c b/libavformat/mov.c >> index 0b1c182..a19ebbf 100644 >> --- a/libavformat/mov.c >> +++ b/libavformat/mov.c >> @@ -1842,6 +1842,7 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, >> AVStream *st, MOVStreamContext *sc) >> { >> uint8_t codec_name[32] = { 0 }; >> + uint8_t vendor[5] = { 0 }; >> int64_t stsd_start; >> unsigned int len; >> >> @@ -1851,7 +1852,9 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, >> >> avio_rb16(pb); /* version */ >> avio_rb16(pb); /* revision level */ >> - avio_rb32(pb); /* vendor */ >> + avio_read(pb, vendor, 4); >> + if (vendor[0]) >> + av_dict_set(&st->metadata, "vendor", vendor, 0); > > Does this mean transcoding to a format with per-stream > tags will add this as a tag? The patch you quoted allows libavformat users to read the vendor tag from mov files, I don't understand how it adds tags. Do you object? Carl Eugen
On Thu, Jan 26, 2017 at 10:04 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-01-25 14:22 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> On Mon, 12 Dec 2016 12:12:37 +0100 >> Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: >> >>> From 7c26220a8734fe7dc293efe6c13e3baf91defc7e Mon Sep 17 00:00:00 2001 >>> From: Carl Eugen Hoyos <cehoyos@ag.or.at> >>> Date: Mon, 12 Dec 2016 12:07:27 +0100 >>> Subject: [PATCH 2/2] lavf/mov: Export vendor metadata. >>> >>> --- >>> libavformat/mov.c | 5 ++++- >>> tests/ref/fate/mov-zombie | 2 +- >>> 2 files changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>> index 0b1c182..a19ebbf 100644 >>> --- a/libavformat/mov.c >>> +++ b/libavformat/mov.c >>> @@ -1842,6 +1842,7 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, >>> AVStream *st, MOVStreamContext *sc) >>> { >>> uint8_t codec_name[32] = { 0 }; >>> + uint8_t vendor[5] = { 0 }; >>> int64_t stsd_start; >>> unsigned int len; >>> >>> @@ -1851,7 +1852,9 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, >>> >>> avio_rb16(pb); /* version */ >>> avio_rb16(pb); /* revision level */ >>> - avio_rb32(pb); /* vendor */ >>> + avio_read(pb, vendor, 4); >>> + if (vendor[0]) >>> + av_dict_set(&st->metadata, "vendor", vendor, 0); >> >> Does this mean transcoding to a format with per-stream >> tags will add this as a tag? > > The patch you quoted allows libavformat users to read the > vendor tag from mov files, I don't understand how it adds > tags. > Do you object? Any metadata you export can and will get copied to a new file when remuxing, therefor exporting arbitrary info that isn't actual stream metadata tags in metadata is problematic - it carries over to the destination file, in which it would be entirely meaningless. - Hendrik
2017-01-26 0:19 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>: > On Thu, Jan 26, 2017 at 10:04 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> 2017-01-25 14:22 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >>> On Mon, 12 Dec 2016 12:12:37 +0100 >>> Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: >>> >>>> From 7c26220a8734fe7dc293efe6c13e3baf91defc7e Mon Sep 17 00:00:00 2001 >>>> From: Carl Eugen Hoyos <cehoyos@ag.or.at> >>>> Date: Mon, 12 Dec 2016 12:07:27 +0100 >>>> Subject: [PATCH 2/2] lavf/mov: Export vendor metadata. >>>> >>>> --- >>>> libavformat/mov.c | 5 ++++- >>>> tests/ref/fate/mov-zombie | 2 +- >>>> 2 files changed, 5 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>>> index 0b1c182..a19ebbf 100644 >>>> --- a/libavformat/mov.c >>>> +++ b/libavformat/mov.c >>>> @@ -1842,6 +1842,7 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, >>>> AVStream *st, MOVStreamContext *sc) >>>> { >>>> uint8_t codec_name[32] = { 0 }; >>>> + uint8_t vendor[5] = { 0 }; >>>> int64_t stsd_start; >>>> unsigned int len; >>>> >>>> @@ -1851,7 +1852,9 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, >>>> >>>> avio_rb16(pb); /* version */ >>>> avio_rb16(pb); /* revision level */ >>>> - avio_rb32(pb); /* vendor */ >>>> + avio_read(pb, vendor, 4); >>>> + if (vendor[0]) >>>> + av_dict_set(&st->metadata, "vendor", vendor, 0); >>> >>> Does this mean transcoding to a format with per-stream >>> tags will add this as a tag? >> >> The patch you quoted allows libavformat users to read the >> vendor tag from mov files, I don't understand how it adds >> tags. >> Do you object? > > Any metadata you export can and will get copied to a new file when > remuxing, therefor exporting arbitrary info that isn't actual stream > metadata tags in metadata is problematic - it carries over to the > destination file, in which it would be entirely meaningless. Sorry, I don't understand: Which application do you mean? Carl Eugen
On Thu, 26 Jan 2017 00:35:17 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-01-26 0:19 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>: > > On Thu, Jan 26, 2017 at 10:04 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2017-01-25 14:22 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >>> On Mon, 12 Dec 2016 12:12:37 +0100 > >>> Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: > >>> > >>>> From 7c26220a8734fe7dc293efe6c13e3baf91defc7e Mon Sep 17 00:00:00 2001 > >>>> From: Carl Eugen Hoyos <cehoyos@ag.or.at> > >>>> Date: Mon, 12 Dec 2016 12:07:27 +0100 > >>>> Subject: [PATCH 2/2] lavf/mov: Export vendor metadata. > >>>> > >>>> --- > >>>> libavformat/mov.c | 5 ++++- > >>>> tests/ref/fate/mov-zombie | 2 +- > >>>> 2 files changed, 5 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/libavformat/mov.c b/libavformat/mov.c > >>>> index 0b1c182..a19ebbf 100644 > >>>> --- a/libavformat/mov.c > >>>> +++ b/libavformat/mov.c > >>>> @@ -1842,6 +1842,7 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, > >>>> AVStream *st, MOVStreamContext *sc) > >>>> { > >>>> uint8_t codec_name[32] = { 0 }; > >>>> + uint8_t vendor[5] = { 0 }; > >>>> int64_t stsd_start; > >>>> unsigned int len; > >>>> > >>>> @@ -1851,7 +1852,9 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, > >>>> > >>>> avio_rb16(pb); /* version */ > >>>> avio_rb16(pb); /* revision level */ > >>>> - avio_rb32(pb); /* vendor */ > >>>> + avio_read(pb, vendor, 4); > >>>> + if (vendor[0]) > >>>> + av_dict_set(&st->metadata, "vendor", vendor, 0); > >>> > >>> Does this mean transcoding to a format with per-stream > >>> tags will add this as a tag? > >> > >> The patch you quoted allows libavformat users to read the > >> vendor tag from mov files, I don't understand how it adds > >> tags. > >> Do you object? > > > > Any metadata you export can and will get copied to a new file when > > remuxing, therefor exporting arbitrary info that isn't actual stream > > metadata tags in metadata is problematic - it carries over to the > > destination file, in which it would be entirely meaningless. > > Sorry, I don't understand: > Which application do you mean? ffmpeg Or does it somehow have a whitelist of which tags to copy? I didn't attempt to confirm the behavior.
2017-01-26 6:24 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > On Thu, 26 Jan 2017 00:35:17 +0100 > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2017-01-26 0:19 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>: >> > On Thu, Jan 26, 2017 at 10:04 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >> 2017-01-25 14:22 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >>> On Mon, 12 Dec 2016 12:12:37 +0100 >> >>> Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: >> >>> >> >>>> From 7c26220a8734fe7dc293efe6c13e3baf91defc7e Mon Sep 17 00:00:00 2001 >> >>>> From: Carl Eugen Hoyos <cehoyos@ag.or.at> >> >>>> Date: Mon, 12 Dec 2016 12:07:27 +0100 >> >>>> Subject: [PATCH 2/2] lavf/mov: Export vendor metadata. >> >>>> >> >>>> --- >> >>>> libavformat/mov.c | 5 ++++- >> >>>> tests/ref/fate/mov-zombie | 2 +- >> >>>> 2 files changed, 5 insertions(+), 2 deletions(-) >> >>>> >> >>>> diff --git a/libavformat/mov.c b/libavformat/mov.c >> >>>> index 0b1c182..a19ebbf 100644 >> >>>> --- a/libavformat/mov.c >> >>>> +++ b/libavformat/mov.c >> >>>> @@ -1842,6 +1842,7 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, >> >>>> AVStream *st, MOVStreamContext *sc) >> >>>> { >> >>>> uint8_t codec_name[32] = { 0 }; >> >>>> + uint8_t vendor[5] = { 0 }; >> >>>> int64_t stsd_start; >> >>>> unsigned int len; >> >>>> >> >>>> @@ -1851,7 +1852,9 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, >> >>>> >> >>>> avio_rb16(pb); /* version */ >> >>>> avio_rb16(pb); /* revision level */ >> >>>> - avio_rb32(pb); /* vendor */ >> >>>> + avio_read(pb, vendor, 4); >> >>>> + if (vendor[0]) >> >>>> + av_dict_set(&st->metadata, "vendor", vendor, 0); >> >>> >> >>> Does this mean transcoding to a format with per-stream >> >>> tags will add this as a tag? >> >> >> >> The patch you quoted allows libavformat users to read the >> >> vendor tag from mov files, I don't understand how it adds >> >> tags. >> >> Do you object? >> > >> > Any metadata you export can and will get copied to a new file when >> > remuxing, therefor exporting arbitrary info that isn't actual stream >> > metadata tags in metadata is problematic - it carries over to the >> > destination file, in which it would be entirely meaningless. >> >> Sorry, I don't understand: >> Which application do you mean? > > ffmpeg Didn't I ping yesterday a patch that avoids this? And wasn't this the mail you answered? Carl Eugen
On Thu, 26 Jan 2017 08:26:02 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-01-26 6:24 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > On Thu, 26 Jan 2017 00:35:17 +0100 > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > >> 2017-01-26 0:19 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>: > >> > On Thu, Jan 26, 2017 at 10:04 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> >> 2017-01-25 14:22 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> >>> On Mon, 12 Dec 2016 12:12:37 +0100 > >> >>> Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: > >> >>> > >> >>>> From 7c26220a8734fe7dc293efe6c13e3baf91defc7e Mon Sep 17 00:00:00 2001 > >> >>>> From: Carl Eugen Hoyos <cehoyos@ag.or.at> > >> >>>> Date: Mon, 12 Dec 2016 12:07:27 +0100 > >> >>>> Subject: [PATCH 2/2] lavf/mov: Export vendor metadata. > >> >>>> > >> >>>> --- > >> >>>> libavformat/mov.c | 5 ++++- > >> >>>> tests/ref/fate/mov-zombie | 2 +- > >> >>>> 2 files changed, 5 insertions(+), 2 deletions(-) > >> >>>> > >> >>>> diff --git a/libavformat/mov.c b/libavformat/mov.c > >> >>>> index 0b1c182..a19ebbf 100644 > >> >>>> --- a/libavformat/mov.c > >> >>>> +++ b/libavformat/mov.c > >> >>>> @@ -1842,6 +1842,7 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, > >> >>>> AVStream *st, MOVStreamContext *sc) > >> >>>> { > >> >>>> uint8_t codec_name[32] = { 0 }; > >> >>>> + uint8_t vendor[5] = { 0 }; > >> >>>> int64_t stsd_start; > >> >>>> unsigned int len; > >> >>>> > >> >>>> @@ -1851,7 +1852,9 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, > >> >>>> > >> >>>> avio_rb16(pb); /* version */ > >> >>>> avio_rb16(pb); /* revision level */ > >> >>>> - avio_rb32(pb); /* vendor */ > >> >>>> + avio_read(pb, vendor, 4); > >> >>>> + if (vendor[0]) > >> >>>> + av_dict_set(&st->metadata, "vendor", vendor, 0); > >> >>> > >> >>> Does this mean transcoding to a format with per-stream > >> >>> tags will add this as a tag? > >> >> > >> >> The patch you quoted allows libavformat users to read the > >> >> vendor tag from mov files, I don't understand how it adds > >> >> tags. > >> >> Do you object? > >> > > >> > Any metadata you export can and will get copied to a new file when > >> > remuxing, therefor exporting arbitrary info that isn't actual stream > >> > metadata tags in metadata is problematic - it carries over to the > >> > destination file, in which it would be entirely meaningless. > >> > >> Sorry, I don't understand: > >> Which application do you mean? > > > > ffmpeg > > Didn't I ping yesterday a patch that avoids this? > And wasn't this the mail you answered? Sorry, I couldn't find it just now. What was the subject line? I'm not necessarily blocking your patch. There are already plenty of other cases where it breaks this way, and your patch just adds another. But maybe they can all be fixed by that patch you mentioned.
2017-01-26 9:07 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> > Any metadata you export can and will get copied to a new file when >> >> > remuxing, therefor exporting arbitrary info that isn't actual stream >> >> > metadata tags in metadata is problematic - it carries over to the >> >> > destination file, in which it would be entirely meaningless. >> >> >> >> Sorry, I don't understand: >> >> Which application do you mean? >> > >> > ffmpeg >> >> Didn't I ping yesterday a patch that avoids this? >> And wasn't this the mail you answered? > > Sorry, I couldn't find it just now. What was the subject line? My mailer (gmail) shows it as the first mail of this thread, so it (hopefully) uses the same subject. > I'm not necessarily blocking your patch. > There are already plenty of other cases where it breaks this > way, Where did you report this? I believe you know how difficult it is to fix bugs that do not get reported. Carl Eugen
On Thu, 26 Jan 2017 09:16:00 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-01-26 9:07 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > >> >> > Any metadata you export can and will get copied to a new file when > >> >> > remuxing, therefor exporting arbitrary info that isn't actual stream > >> >> > metadata tags in metadata is problematic - it carries over to the > >> >> > destination file, in which it would be entirely meaningless. > >> >> > >> >> Sorry, I don't understand: > >> >> Which application do you mean? > >> > > >> > ffmpeg > >> > >> Didn't I ping yesterday a patch that avoids this? > >> And wasn't this the mail you answered? > > > > Sorry, I couldn't find it just now. What was the subject line? > > My mailer (gmail) shows it as the first mail of this thread, so > it (hopefully) uses the same subject. Sorry, I missed that. But this code is not called for remuxing, or is it? > > I'm not necessarily blocking your patch. > > > There are already plenty of other cases where it breaks this > > way, > > Where did you report this? > I believe you know how difficult it is to fix bugs that do not get > reported. I think I mentioned it multiple times. I do not report ffmpeg bugs because I'm more productive not doing that, sorry.
2017-01-26 9:26 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > On Thu, 26 Jan 2017 09:16:00 +0100 > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2017-01-26 9:07 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> >> >> > Any metadata you export can and will get copied to a new file when >> >> >> > remuxing, therefor exporting arbitrary info that isn't actual stream >> >> >> > metadata tags in metadata is problematic - it carries over to the >> >> >> > destination file, in which it would be entirely meaningless. >> >> >> >> >> >> Sorry, I don't understand: >> >> >> Which application do you mean? >> >> > >> >> > ffmpeg >> >> >> >> Didn't I ping yesterday a patch that avoids this? >> >> And wasn't this the mail you answered? >> > >> > Sorry, I couldn't find it just now. What was the subject line? >> >> My mailer (gmail) shows it as the first mail of this thread, so >> it (hopefully) uses the same subject. > > Sorry, I missed that. But this code is not called for remuxing, > or is it? Afair, this behaviour (copying the vendor on remuxing) is consistent with the Apple documentation. Carl Eugen
On Thu, 26 Jan 2017 18:06:39 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-01-26 9:26 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > On Thu, 26 Jan 2017 09:16:00 +0100 > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > >> 2017-01-26 9:07 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> > >> >> >> > Any metadata you export can and will get copied to a new file when > >> >> >> > remuxing, therefor exporting arbitrary info that isn't actual stream > >> >> >> > metadata tags in metadata is problematic - it carries over to the > >> >> >> > destination file, in which it would be entirely meaningless. > >> >> >> > >> >> >> Sorry, I don't understand: > >> >> >> Which application do you mean? > >> >> > > >> >> > ffmpeg > >> >> > >> >> Didn't I ping yesterday a patch that avoids this? > >> >> And wasn't this the mail you answered? > >> > > >> > Sorry, I couldn't find it just now. What was the subject line? > >> > >> My mailer (gmail) shows it as the first mail of this thread, so > >> it (hopefully) uses the same subject. > > > > Sorry, I missed that. But this code is not called for remuxing, > > or is it? > > Afair, this behaviour (copying the vendor on remuxing) is > consistent with the Apple documentation. > What if you remux to mkv?
2017-01-27 7:04 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > On Thu, 26 Jan 2017 18:06:39 +0100 > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2017-01-26 9:26 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> > On Thu, 26 Jan 2017 09:16:00 +0100 >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> > >> >> 2017-01-26 9:07 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> >> >> >> >> > Any metadata you export can and will get copied to a new file when >> >> >> >> > remuxing, therefor exporting arbitrary info that isn't actual stream >> >> >> >> > metadata tags in metadata is problematic - it carries over to the >> >> >> >> > destination file, in which it would be entirely meaningless. >> >> >> >> >> >> >> >> Sorry, I don't understand: >> >> >> >> Which application do you mean? >> >> >> > >> >> >> > ffmpeg >> >> >> >> >> >> Didn't I ping yesterday a patch that avoids this? >> >> >> And wasn't this the mail you answered? >> >> > >> >> > Sorry, I couldn't find it just now. What was the subject line? >> >> >> >> My mailer (gmail) shows it as the first mail of this thread, so >> >> it (hopefully) uses the same subject. >> > >> > Sorry, I missed that. But this code is not called for remuxing, >> > or is it? >> >> Afair, this behaviour (copying the vendor on remuxing) is >> consistent with the Apple documentation. > > What if you remux to mkv? Afair, copying the vendor tag on remuxing is consistent with its documentation. Carl Eugen
On Fri, 27 Jan 2017 08:26:26 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-01-27 7:04 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > On Thu, 26 Jan 2017 18:06:39 +0100 > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > >> 2017-01-26 9:26 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> > On Thu, 26 Jan 2017 09:16:00 +0100 > >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> > > >> >> 2017-01-26 9:07 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> >> > >> >> >> >> > Any metadata you export can and will get copied to a new file when > >> >> >> >> > remuxing, therefor exporting arbitrary info that isn't actual stream > >> >> >> >> > metadata tags in metadata is problematic - it carries over to the > >> >> >> >> > destination file, in which it would be entirely meaningless. > >> >> >> >> > >> >> >> >> Sorry, I don't understand: > >> >> >> >> Which application do you mean? > >> >> >> > > >> >> >> > ffmpeg > >> >> >> > >> >> >> Didn't I ping yesterday a patch that avoids this? > >> >> >> And wasn't this the mail you answered? > >> >> > > >> >> > Sorry, I couldn't find it just now. What was the subject line? > >> >> > >> >> My mailer (gmail) shows it as the first mail of this thread, so > >> >> it (hopefully) uses the same subject. > >> > > >> > Sorry, I missed that. But this code is not called for remuxing, > >> > or is it? > >> > >> Afair, this behaviour (copying the vendor on remuxing) is > >> consistent with the Apple documentation. > > > > What if you remux to mkv? > > Afair, copying the vendor tag on remuxing is consistent with > its documentation. Well, the Apple docs don't have anything to do with mkv?
2017-01-27 8:56 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > On Fri, 27 Jan 2017 08:26:26 +0100 > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2017-01-27 7:04 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> > On Thu, 26 Jan 2017 18:06:39 +0100 >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> > >> >> 2017-01-26 9:26 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> > On Thu, 26 Jan 2017 09:16:00 +0100 >> >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >> > >> >> >> 2017-01-26 9:07 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> >> >> >> >> >> >> > Any metadata you export can and will get copied to a new file when >> >> >> >> >> > remuxing, therefor exporting arbitrary info that isn't actual stream >> >> >> >> >> > metadata tags in metadata is problematic - it carries over to the >> >> >> >> >> > destination file, in which it would be entirely meaningless. >> >> >> >> >> >> >> >> >> >> Sorry, I don't understand: >> >> >> >> >> Which application do you mean? >> >> >> >> > >> >> >> >> > ffmpeg >> >> >> >> >> >> >> >> Didn't I ping yesterday a patch that avoids this? >> >> >> >> And wasn't this the mail you answered? >> >> >> > >> >> >> > Sorry, I couldn't find it just now. What was the subject line? >> >> >> >> >> >> My mailer (gmail) shows it as the first mail of this thread, so >> >> >> it (hopefully) uses the same subject. >> >> > >> >> > Sorry, I missed that. But this code is not called for remuxing, >> >> > or is it? >> >> >> >> Afair, this behaviour (copying the vendor on remuxing) is >> >> consistent with the Apple documentation. >> > >> > What if you remux to mkv? >> >> Afair, copying the vendor tag on remuxing is consistent with >> its documentation. > > Well, the Apple docs don't have anything to do with mkv? So mkv has a specific definition of the vendor metadata? I did not immediately find it, could you point me to it? Carl Eugen
On Fri, 27 Jan 2017 09:05:15 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-01-27 8:56 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > On Fri, 27 Jan 2017 08:26:26 +0100 > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > >> 2017-01-27 7:04 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> > On Thu, 26 Jan 2017 18:06:39 +0100 > >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> > > >> >> 2017-01-26 9:26 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> >> > On Thu, 26 Jan 2017 09:16:00 +0100 > >> >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> >> > > >> >> >> 2017-01-26 9:07 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> >> >> > >> >> >> >> >> > Any metadata you export can and will get copied to a new file when > >> >> >> >> >> > remuxing, therefor exporting arbitrary info that isn't actual stream > >> >> >> >> >> > metadata tags in metadata is problematic - it carries over to the > >> >> >> >> >> > destination file, in which it would be entirely meaningless. > >> >> >> >> >> > >> >> >> >> >> Sorry, I don't understand: > >> >> >> >> >> Which application do you mean? > >> >> >> >> > > >> >> >> >> > ffmpeg > >> >> >> >> > >> >> >> >> Didn't I ping yesterday a patch that avoids this? > >> >> >> >> And wasn't this the mail you answered? > >> >> >> > > >> >> >> > Sorry, I couldn't find it just now. What was the subject line? > >> >> >> > >> >> >> My mailer (gmail) shows it as the first mail of this thread, so > >> >> >> it (hopefully) uses the same subject. > >> >> > > >> >> > Sorry, I missed that. But this code is not called for remuxing, > >> >> > or is it? > >> >> > >> >> Afair, this behaviour (copying the vendor on remuxing) is > >> >> consistent with the Apple documentation. > >> > > >> > What if you remux to mkv? > >> > >> Afair, copying the vendor tag on remuxing is consistent with > >> its documentation. > > > > Well, the Apple docs don't have anything to do with mkv? > > So mkv has a specific definition of the vendor metadata? > I did not immediately find it, could you point me to it? You're completely misunderstanding.
2017-01-27 9:17 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
> You're completely misunderstanding.
Would you mind to elaborate?
Carl Eugen
On Fri, 27 Jan 2017 09:39:03 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-01-27 9:17 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > > You're completely misunderstanding. > > Would you mind to elaborate? FFmpeg shouldn't mux codec-specific tags into a different container. Your ffmpeg.c patch works for transcoding only, not remuxing. Also, an API user can't distinguish user tags and container-specific metadata added by libavformat, which is a problem. In theory, you have to add this information as side-data. Adding a new side-data type is a bit roundabout, though.
2017-01-27 10:09 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > On Fri, 27 Jan 2017 09:39:03 +0100 > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2017-01-27 9:17 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> > You're completely misunderstanding. >> >> Would you mind to elaborate? > > FFmpeg shouldn't mux codec-specific tags into a different > container. This is not codec-specific, at least not in the sense that would make a difference for other containers. > Your ffmpeg.c patch works for transcoding only, not remuxing. That's because it makes sense to remux "vendor" metadata. Carl Eugen
On Fri, 27 Jan 2017 10:19:32 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-01-27 10:09 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > On Fri, 27 Jan 2017 09:39:03 +0100 > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > >> 2017-01-27 9:17 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> > >> > You're completely misunderstanding. > >> > >> Would you mind to elaborate? > > > > FFmpeg shouldn't mux codec-specific tags into a different > > container. > > This is not codec-specific, at least not in the sense that > would make a difference for other containers. > > > Your ffmpeg.c patch works for transcoding only, not remuxing. > > That's because it makes sense to remux "vendor" metadata. No, technical metadata that is "hidden" is different from user-visible tags that contain non-technical information about the media. Don't mix them. FFmpeg does, and this is bad.
2017-01-27 10:29 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > On Fri, 27 Jan 2017 10:19:32 +0100 > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2017-01-27 10:09 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> > On Fri, 27 Jan 2017 09:39:03 +0100 >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> > >> >> 2017-01-27 9:17 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> >> >> > You're completely misunderstanding. >> >> >> >> Would you mind to elaborate? >> > >> > FFmpeg shouldn't mux codec-specific tags into a different >> > container. >> >> This is not codec-specific, at least not in the sense that >> would make a difference for other containers. >> >> > Your ffmpeg.c patch works for transcoding only, not remuxing. >> >> That's because it makes sense to remux "vendor" metadata. > > No, technical metadata that is "hidden" is different from user-visible > tags that contain non-technical information about the media. This is non-technical information that should be user-visible. Carl Eugen
On Fri, 27 Jan 2017 10:38:23 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-01-27 10:29 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > On Fri, 27 Jan 2017 10:19:32 +0100 > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > >> 2017-01-27 10:09 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> > On Fri, 27 Jan 2017 09:39:03 +0100 > >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> > > >> >> 2017-01-27 9:17 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> >> > >> >> > You're completely misunderstanding. > >> >> > >> >> Would you mind to elaborate? > >> > > >> > FFmpeg shouldn't mux codec-specific tags into a different > >> > container. > >> > >> This is not codec-specific, at least not in the sense that > >> would make a difference for other containers. > >> > >> > Your ffmpeg.c patch works for transcoding only, not remuxing. > >> > >> That's because it makes sense to remux "vendor" metadata. > > > > No, technical metadata that is "hidden" is different from user-visible > > tags that contain non-technical information about the media. > > This is non-technical information that should be user-visible. The vendor tag in your fate change has the value "appl". How should that be user-visible? You can't seriously tell me that this should show up in a non-technical UI view.
On 12/12/16, Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: > Hi! > > I saw only after writing this patch that a Google employee has sent > a more complicated variant of this patch in November (searching for > a description of the vendor field showed the patch). > > Please comment, Carl Eugen > -1
2017-01-27 10:42 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > On Fri, 27 Jan 2017 10:38:23 +0100 > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2017-01-27 10:29 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> > On Fri, 27 Jan 2017 10:19:32 +0100 >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> > >> >> 2017-01-27 10:09 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> > On Fri, 27 Jan 2017 09:39:03 +0100 >> >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >> > >> >> >> 2017-01-27 9:17 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> >> >> >> >> > You're completely misunderstanding. >> >> >> >> >> >> Would you mind to elaborate? >> >> > >> >> > FFmpeg shouldn't mux codec-specific tags into a different >> >> > container. >> >> >> >> This is not codec-specific, at least not in the sense that >> >> would make a difference for other containers. >> >> >> >> > Your ffmpeg.c patch works for transcoding only, not remuxing. >> >> >> >> That's because it makes sense to remux "vendor" metadata. >> > >> > No, technical metadata that is "hidden" is different from user-visible >> > tags that contain non-technical information about the media. >> >> This is non-technical information that should be user-visible. > > The vendor tag in your fate change has the value "appl". How should > that be user-visible? You mean the information is encoded in such a difficult manner that no user will be able to decipher it? I really wish you were a little more serious when trying to slow down FFmpeg development. > You can't seriously tell me that this should show > up in a non-technical UI view. Since you are blocking much more important patches, I should probably not spend so much time discussing this one. But the fact that I needed this patch for debugging and that I found out afterwards that other developers need it too sufficiently indicates to me showing, exporting and remuxing the vendor tag makes sense. Carl Eugen
On Fri, 27 Jan 2017 11:21:08 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-01-27 10:42 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > On Fri, 27 Jan 2017 10:38:23 +0100 > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > >> 2017-01-27 10:29 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> > On Fri, 27 Jan 2017 10:19:32 +0100 > >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> > > >> >> 2017-01-27 10:09 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> >> > On Fri, 27 Jan 2017 09:39:03 +0100 > >> >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> >> > > >> >> >> 2017-01-27 9:17 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> >> >> > >> >> >> > You're completely misunderstanding. > >> >> >> > >> >> >> Would you mind to elaborate? > >> >> > > >> >> > FFmpeg shouldn't mux codec-specific tags into a different > >> >> > container. > >> >> > >> >> This is not codec-specific, at least not in the sense that > >> >> would make a difference for other containers. > >> >> > >> >> > Your ffmpeg.c patch works for transcoding only, not remuxing. > >> >> > >> >> That's because it makes sense to remux "vendor" metadata. > >> > > >> > No, technical metadata that is "hidden" is different from user-visible > >> > tags that contain non-technical information about the media. > >> > >> This is non-technical information that should be user-visible. > > > > The vendor tag in your fate change has the value "appl". How should > > that be user-visible? > > You mean the information is encoded in such a difficult manner > that no user will be able to decipher it? That too. It's essentially a FourCC. Most people don't know what to do with FourCC. It's something for developers or otherwise technically inclined people. While this strengthens my argument, it's not really the main point. > I really wish you were a little more serious when trying to slow down > FFmpeg development. Please refrain from personal attacks. I'm not trying to slow down development, I'm trying to improve the general quality of FFmpeg. In fact, that's the main point of patch reviews: point out weak spots, request improvements, and so on. In this specific case, you're exporting a technical details as user tags. This is a practice that should stop. Also, writing essentially random garbage as tags or writing it to files in a manner that doesn't make sense is not what I associate with quality. What sense does it make to write a stringified FourCC as user-tag to, say, a mkv file? Last but not least, it's questionable whether this is a meaningful feature at all. Why not use a tool that's more suited for the job, like a mp4-specific file dumper? (Why did you not do the same for audio?) To put this into perspective, mediainfo -f doesn't seem to export this value. > > You can't seriously tell me that this should show > > up in a non-technical UI view. > > Since you are blocking much more important patches, I should > probably not spend so much time discussing this one. But the > fact that I needed this patch for debugging and that I found out > afterwards that other developers need it too sufficiently indicates > to me showing, exporting and remuxing the vendor tag makes sense. For debugging you probably want to use a tool like boxdumper. FFmpeg can't and won't be able to do "everything" a more suited tool can do, because it's simply out of scope, and would flood us with thousands of normally useless details. I'm also sad to see that you consider discussing patches an annoyance. Besides, aren't you one of the main offenders when it comes to stubbornly blocking certain changes?
2017-01-27 11:55 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > On Fri, 27 Jan 2017 11:21:08 +0100 > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2017-01-27 10:42 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> > On Fri, 27 Jan 2017 10:38:23 +0100 >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> > >> >> 2017-01-27 10:29 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> > On Fri, 27 Jan 2017 10:19:32 +0100 >> >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >> > >> >> >> 2017-01-27 10:09 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> >> > On Fri, 27 Jan 2017 09:39:03 +0100 >> >> >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >> >> > >> >> >> >> 2017-01-27 9:17 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> >> >> >> >> >> >> > You're completely misunderstanding. >> >> >> >> >> >> >> >> Would you mind to elaborate? >> >> >> > >> >> >> > FFmpeg shouldn't mux codec-specific tags into a different >> >> >> > container. >> >> >> >> >> >> This is not codec-specific, at least not in the sense that >> >> >> would make a difference for other containers. >> >> >> >> >> >> > Your ffmpeg.c patch works for transcoding only, not remuxing. >> >> >> >> >> >> That's because it makes sense to remux "vendor" metadata. >> >> > >> >> > No, technical metadata that is "hidden" is different from user-visible >> >> > tags that contain non-technical information about the media. >> >> >> >> This is non-technical information that should be user-visible. >> > >> > The vendor tag in your fate change has the value "appl". How should >> > that be user-visible? >> >> You mean the information is encoded in such a difficult manner >> that no user will be able to decipher it? > > That too. It's essentially a FourCC. Most people don't know what to do > with FourCC. It's something for developers or otherwise technically > inclined people. I disagree. (And so do other developers it seems.) > While this strengthens my argument, it's not really the main point. > >> I really wish you were a little more serious when trying to slow down >> FFmpeg development. > > Please refrain from personal attacks. I do not consider this a personal attack: People you seem to respect have often requested (publically and in private) that FFmpeg development has to slow down - I strongly disagree with them but you have said in the past that you agree with them. > I'm not trying to slow down > development, I'm trying to improve the general quality of FFmpeg. In > fact, that's the main point of patch reviews: point out weak spots, > request improvements, and so on. But that is not how you started this thread, or was it? And that's apart from the fact that you have threatened contributors away in the past. > In this specific case, you're exporting a technical details as > user tags. This is a practice that should stop. Also, writing > essentially random garbage as tags or writing it to files in a manner > that doesn't make sense is not what I associate with quality. What > sense does it make to write a stringified FourCC as user-tag to, say, > a mkv file? I do not understand in which way the vendor tag "doesn't make sense" in any container. > Last but not least, it's questionable whether this is a meaningful > feature at all. Why not use a tool that's more suited for the job, like > a mp4-specific file dumper? (Why did you not do the same for audio?) I would really, really prefer if all necessary debug information would be printed by FFmpeg, not a third-party toold. > To put this into perspective, mediainfo -f doesn't seem to export this > value. This does not sound like a useful argument to me. >> > You can't seriously tell me that this should show >> > up in a non-technical UI view. >> >> Since you are blocking much more important patches, I should >> probably not spend so much time discussing this one. But the >> fact that I needed this patch for debugging and that I found out >> afterwards that other developers need it too sufficiently indicates >> to me showing, exporting and remuxing the vendor tag makes sense. > > For debugging you probably want to use a tool like boxdumper. See above. > FFmpeg can't and won't be able to do "everything" a more suited tool > can do, because it's simply out of scope, and would flood us with > thousands of normally useless details. How is exporting properties of a media file out-of-scope for FFmpeg? > I'm also sad to see that you consider discussing patches an annoyance. I wish you would have commented earlier, the way you did it gave me the impression you were only interested in annoying me. > Besides, aren't you one of the main offenders when it comes to > stubbornly blocking certain changes? Compared to you? Carl Eugen
On Fri, 27 Jan 2017 12:35:25 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-01-27 11:55 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > On Fri, 27 Jan 2017 11:21:08 +0100 > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > >> 2017-01-27 10:42 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> > On Fri, 27 Jan 2017 10:38:23 +0100 > >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> > > >> >> 2017-01-27 10:29 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> >> > On Fri, 27 Jan 2017 10:19:32 +0100 > >> >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> >> > > >> >> >> 2017-01-27 10:09 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> >> >> > On Fri, 27 Jan 2017 09:39:03 +0100 > >> >> >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> >> >> > > >> >> >> >> 2017-01-27 9:17 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> >> >> >> > >> >> >> >> > You're completely misunderstanding. > >> >> >> >> > >> >> >> >> Would you mind to elaborate? > >> >> >> > > >> >> >> > FFmpeg shouldn't mux codec-specific tags into a different > >> >> >> > container. > >> >> >> > >> >> >> This is not codec-specific, at least not in the sense that > >> >> >> would make a difference for other containers. > >> >> >> > >> >> >> > Your ffmpeg.c patch works for transcoding only, not remuxing. > >> >> >> > >> >> >> That's because it makes sense to remux "vendor" metadata. > >> >> > > >> >> > No, technical metadata that is "hidden" is different from user-visible > >> >> > tags that contain non-technical information about the media. > >> >> > >> >> This is non-technical information that should be user-visible. > >> > > >> > The vendor tag in your fate change has the value "appl". How should > >> > that be user-visible? > >> > >> You mean the information is encoded in such a difficult manner > >> that no user will be able to decipher it? > > > > That too. It's essentially a FourCC. Most people don't know what to do > > with FourCC. It's something for developers or otherwise technically > > inclined people. > > I disagree. > (And so do other developers it seems.) Which other developers? > > While this strengthens my argument, it's not really the main point. > > > >> I really wish you were a little more serious when trying to slow down > >> FFmpeg development. > > > > Please refrain from personal attacks. > > I do not consider this a personal attack: People you seem to respect > have often requested (publically and in private) that FFmpeg development > has to slow down - I strongly disagree with them but you have said in the > past that you agree with them. Where did they say this? Do you have a link or an example? > > I'm not trying to slow down > > development, I'm trying to improve the general quality of FFmpeg. In > > fact, that's the main point of patch reviews: point out weak spots, > > request improvements, and so on. > > But that is not how you started this thread, or was it? Did you? > And that's apart from the fact that you have threatened contributors > away in the past. How many people have you chased away from the bug tracker? > > In this specific case, you're exporting a technical details as > > user tags. This is a practice that should stop. Also, writing > > essentially random garbage as tags or writing it to files in a manner > > that doesn't make sense is not what I associate with quality. What > > sense does it make to write a stringified FourCC as user-tag to, say, > > a mkv file? > > I do not understand in which way the vendor tag "doesn't make sense" > in any container. See the paragraph you quoted. Let me ask you again: how does an Apple-specific FourCC make sense in mkv (except maybe the Qt muxing support)? > > Last but not least, it's questionable whether this is a meaningful > > feature at all. Why not use a tool that's more suited for the job, like > > a mp4-specific file dumper? (Why did you not do the same for audio?) > > I would really, really prefer if all necessary debug information would be > printed by FFmpeg, not a third-party toold. Adding user-visible changes "for debugging" is usually a bad argument. You could output it with an av_log with debug log level if you must. > > To put this into perspective, mediainfo -f doesn't seem to export this > > value. > > This does not sound like a useful argument to me. It does to me. > >> > You can't seriously tell me that this should show > >> > up in a non-technical UI view. > >> > >> Since you are blocking much more important patches, I should > >> probably not spend so much time discussing this one. But the > >> fact that I needed this patch for debugging and that I found out > >> afterwards that other developers need it too sufficiently indicates > >> to me showing, exporting and remuxing the vendor tag makes sense. > > > > For debugging you probably want to use a tool like boxdumper. > > See above. See above. > > FFmpeg can't and won't be able to do "everything" a more suited tool > > can do, because it's simply out of scope, and would flood us with > > thousands of normally useless details. > > How is exporting properties of a media file out-of-scope for FFmpeg? Don't twist my words. I wasn't talking about "exporting properties", but this specific case. > > I'm also sad to see that you consider discussing patches an annoyance. > > I wish you would have commented earlier, the way you did it gave me > the impression you were only interested in annoying me. Oh look, a personal attack mixed with plenty of mind-bending bullshit again. > > Besides, aren't you one of the main offenders when it comes to > > stubbornly blocking certain changes? > > Compared to you? I'm less stubborn than you. Because I give in when I see that I'm wrong, or if the opposition is too large.
On Mon, Dec 12, 2016 at 12:12:37PM +0100, Carl Eugen Hoyos wrote: > Hi! > > I saw only after writing this patch that a Google employee has sent > a more complicated variant of this patch in November (searching for > a description of the vendor field showed the patch). > > Please comment, Carl Eugen It seems some developers dislike exporting 4 byte vendor tags as metadata. Maybe using the table from: http://www.sno.phy.queensu.ca/~phil/exiftool/TagNames/QuickTime.html#VendorID And instead exporting the full vendor name as metadata from the demuxer and converting it back to a 4byte value using the same table in the muxer would be an option ? If this doesnt result in a resolution of the disagreements then feel free to ignore my suggestion, its not my intent to complicate this by adding additional requests thx [...]
diff --git a/ffmpeg.c b/ffmpeg.c index e4890a4..f58e9b1 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -3014,6 +3014,7 @@ static void set_encoder_id(OutputFile *of, OutputStream *ost) av_strlcat(encoder_string, ost->enc->name, encoder_string_len); av_dict_set(&ost->st->metadata, "encoder", encoder_string, AV_DICT_DONT_STRDUP_VAL | AV_DICT_DONT_OVERWRITE); + av_dict_set(&ost->st->metadata, "vendor", NULL, 0); } static void parse_forced_key_frames(char *kf, OutputStream *ost,
Hi! I saw only after writing this patch that a Google employee has sent a more complicated variant of this patch in November (searching for a description of the vendor field showed the patch). Please comment, Carl Eugen From 182ee7b3dca51a49a606c4d11758819ec29fc181 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <cehoyos@ag.or.at> Date: Mon, 12 Dec 2016 12:05:38 +0100 Subject: [PATCH 1/2] ffmpeg: Do not copy vendor metadata on reencoding. --- ffmpeg.c | 1 + 1 file changed, 1 insertion(+)