| Message ID | 20161202200057.6685-1-vittorio.giovara@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 12/2/2016 5:00 PM, Vittorio Giovara wrote: > This will simplify identifying files that were generated with > unfinished/incomplete/non-standard specifications. > > Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com> > --- > libavformat/movenc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index dc19838..c46bea9 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -5756,6 +5756,7 @@ static int mov_init(AVFormatContext *s) > FF_COMPLIANCE_EXPERIMENTAL); > return AVERROR_EXPERIMENTAL; > } > + av_dict_set(&s->metadata, "WARNING", "This file was generated using an unfinished specification, please don't modify your demuxer to support it, should it not work", 0); > } > } else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { > track->timescale = st->codecpar->sample_rate; > @@ -5802,6 +5803,7 @@ static int mov_init(AVFormatContext *s) > FF_COMPLIANCE_EXPERIMENTAL); > return AVERROR_EXPERIMENTAL; > } > + av_dict_set(&s->metadata, "WARNING", "This file was generated using an unfinished specification, please don't modify your demuxer to support it, should it not work", 0); > } > } else if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) { > track->timescale = st->time_base.den; Maybe lavf and lavc should add "experimental" to the container and stream's metadata alongside the library version, much like how we're adding the name of the encoder alongside the lavc version to the stream's metadata. A warning printed by the muxer not going to help once such a file is in the wild.
On Fri, 2 Dec 2016 17:17:23 -0300 James Almer <jamrial@gmail.com> wrote: > On 12/2/2016 5:00 PM, Vittorio Giovara wrote: > > This will simplify identifying files that were generated with > > unfinished/incomplete/non-standard specifications. > > > > Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com> > > --- > > libavformat/movenc.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > > index dc19838..c46bea9 100644 > > --- a/libavformat/movenc.c > > +++ b/libavformat/movenc.c > > @@ -5756,6 +5756,7 @@ static int mov_init(AVFormatContext *s) > > FF_COMPLIANCE_EXPERIMENTAL); > > return AVERROR_EXPERIMENTAL; > > } > > + av_dict_set(&s->metadata, "WARNING", "This file was generated using an unfinished specification, please don't modify your demuxer to support it, should it not work", 0); > > } > > } else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { > > track->timescale = st->codecpar->sample_rate; > > @@ -5802,6 +5803,7 @@ static int mov_init(AVFormatContext *s) > > FF_COMPLIANCE_EXPERIMENTAL); > > return AVERROR_EXPERIMENTAL; > > } > > + av_dict_set(&s->metadata, "WARNING", "This file was generated using an unfinished specification, please don't modify your demuxer to support it, should it not work", 0); > > } > > } else if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) { > > track->timescale = st->time_base.den; > > Maybe lavf and lavc should add "experimental" to the container and > stream's metadata alongside the library version, much like how we're > adding the name of the encoder alongside the lavc version to the > stream's metadata. > > A warning printed by the muxer not going to help once such a file > is in the wild. This patch or the suggestion above sounds like a good idea. I wish it'd actually be possible to encode a watermark with the warning into the video stream.
On Fri, Dec 2, 2016 at 3:00 PM, Vittorio Giovara <vittorio.giovara@gmail.com> wrote: > This will simplify identifying files that were generated with > unfinished/incomplete/non-standard specifications. > > Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com> > --- > libavformat/movenc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index dc19838..c46bea9 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -5756,6 +5756,7 @@ static int mov_init(AVFormatContext *s) > FF_COMPLIANCE_EXPERIMENTAL); > return AVERROR_EXPERIMENTAL; > } > + av_dict_set(&s->metadata, "WARNING", "This file was generated using an unfinished specification, please don't modify your demuxer to support it, should it not work", 0); > } > } else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { > track->timescale = st->codecpar->sample_rate; > @@ -5802,6 +5803,7 @@ static int mov_init(AVFormatContext *s) > FF_COMPLIANCE_EXPERIMENTAL); > return AVERROR_EXPERIMENTAL; > } > + av_dict_set(&s->metadata, "WARNING", "This file was generated using an unfinished specification, please don't modify your demuxer to support it, should it not work", 0); > } > } else if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) { > track->timescale = st->time_base.den; > -- Any opinion about this? Should I assume it is fine as is? Please CC.
On 12/2/2016 5:17 PM, James Almer wrote: > On 12/2/2016 5:00 PM, Vittorio Giovara wrote: >> This will simplify identifying files that were generated with >> unfinished/incomplete/non-standard specifications. >> >> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com> >> --- >> libavformat/movenc.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >> index dc19838..c46bea9 100644 >> --- a/libavformat/movenc.c >> +++ b/libavformat/movenc.c >> @@ -5756,6 +5756,7 @@ static int mov_init(AVFormatContext *s) >> FF_COMPLIANCE_EXPERIMENTAL); >> return AVERROR_EXPERIMENTAL; >> } >> + av_dict_set(&s->metadata, "WARNING", "This file was generated using an unfinished specification, please don't modify your demuxer to support it, should it not work", 0); >> } >> } else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { >> track->timescale = st->codecpar->sample_rate; >> @@ -5802,6 +5803,7 @@ static int mov_init(AVFormatContext *s) >> FF_COMPLIANCE_EXPERIMENTAL); >> return AVERROR_EXPERIMENTAL; >> } >> + av_dict_set(&s->metadata, "WARNING", "This file was generated using an unfinished specification, please don't modify your demuxer to support it, should it not work", 0); >> } >> } else if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) { >> track->timescale = st->time_base.den; > > Maybe lavf and lavc should add "experimental" to the container and > stream's metadata alongside the library version, much like how we're > adding the name of the encoder alongside the lavc version to the > stream's metadata. > > A warning printed by the muxer not going to help once such a file > is in the wild. Resending, CCing Vittorio this time.
On Tue, Dec 6, 2016 at 10:50 AM, James Almer <jamrial@gmail.com> wrote: > On 12/2/2016 5:17 PM, James Almer wrote: >> On 12/2/2016 5:00 PM, Vittorio Giovara wrote: >>> This will simplify identifying files that were generated with >>> unfinished/incomplete/non-standard specifications. >>> >>> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com> >>> --- >>> libavformat/movenc.c | 2 ++ >>> 1 file changed, 2 insertions(+) >> >> Maybe lavf and lavc should add "experimental" to the container and >> stream's metadata alongside the library version, much like how we're >> adding the name of the encoder alongside the lavc version to the >> stream's metadata. >> >> A warning printed by the muxer not going to help once such a file >> is in the wild. Yeah I was unsure about that -- are you saying that whenever -strict experimental is used we should add this tag regardless? The idea behind this is that in the future, when the standard is consolidated, if there is a file is in the wild encoded with an "experimental" codec/container combination and it is not playing correctly, this tag will make evident that the bug lies within the file and the playback issues are probably due the unfinished state of the specification, rather than in in the modern implementation of the demuxer. In other words, people will be less inclined to hack around their own demuxer to accommodate for broken files.
On 12/6/2016 2:31 PM, Vittorio Giovara wrote: > On Tue, Dec 6, 2016 at 10:50 AM, James Almer <jamrial@gmail.com> wrote: >> On 12/2/2016 5:17 PM, James Almer wrote: >>> On 12/2/2016 5:00 PM, Vittorio Giovara wrote: >>>> This will simplify identifying files that were generated with >>>> unfinished/incomplete/non-standard specifications. >>>> >>>> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com> >>>> --- >>>> libavformat/movenc.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>> >>> Maybe lavf and lavc should add "experimental" to the container and >>> stream's metadata alongside the library version, much like how we're >>> adding the name of the encoder alongside the lavc version to the >>> stream's metadata. >>> >>> A warning printed by the muxer not going to help once such a file >>> is in the wild. > > Yeah I was unsure about that -- are you saying that whenever -strict > experimental is used we should add this tag regardless? > > The idea behind this is that in the future, when the standard is > consolidated, if there is a file is in the wild encoded with an > "experimental" codec/container combination and it is not playing > correctly, this tag will make evident that the bug lies within the > file and the playback issues are probably due the unfinished state of > the specification, rather than in in the modern implementation of the > demuxer. In other words, people will be less inclined to hack around > their own demuxer to accommodate for broken files. Oh, i misread the patch. I thought it was printing a warning, not adding a "WARNING" stream metadata tag. This patch is ok as is, i guess, but maybe a more general solution like what i suggested is better. Having the standard "encoder" tags read something like "Lavf5x.xx.xx (experimental)" and "Lavc5x.xx.xx vorbis (experimental)" if -strict experimental is used regardless of format, instead of a non standard "WARNING" tag in one or two specific containers, which may or may not be ignored by players and parsers.
2016-12-02 21:00 GMT+01:00 Vittorio Giovara <vittorio.giovara@gmail.com>: > This will simplify identifying files that were generated with > unfinished/incomplete/non-standard specifications. Doesn't the muxer version information already provide this information? Are random metadata tags valid? Carl Eugen
diff --git a/libavformat/movenc.c b/libavformat/movenc.c index dc19838..c46bea9 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -5756,6 +5756,7 @@ static int mov_init(AVFormatContext *s) FF_COMPLIANCE_EXPERIMENTAL); return AVERROR_EXPERIMENTAL; } + av_dict_set(&s->metadata, "WARNING", "This file was generated using an unfinished specification, please don't modify your demuxer to support it, should it not work", 0); } } else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { track->timescale = st->codecpar->sample_rate; @@ -5802,6 +5803,7 @@ static int mov_init(AVFormatContext *s) FF_COMPLIANCE_EXPERIMENTAL); return AVERROR_EXPERIMENTAL; } + av_dict_set(&s->metadata, "WARNING", "This file was generated using an unfinished specification, please don't modify your demuxer to support it, should it not work", 0); } } else if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) { track->timescale = st->time_base.den;
This will simplify identifying files that were generated with unfinished/incomplete/non-standard specifications. Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com> --- libavformat/movenc.c | 2 ++ 1 file changed, 2 insertions(+)