| Message ID | 20190823234727.27935-1-cus@passwd.hu |
|---|---|
| State | New |
| Headers | show |
Am Sa., 24. Aug. 2019 um 01:47 Uhr schrieb Marton Balint <cus@passwd.hu>: > > Some security cams generate this, as well as some versions of VirtualDub so > support for _reading_ such files is justified. Please also mention vlc. > > Fixes ticket #7110. > > See also this discussion: https://patchwork.ffmpeg.org/patch/8744/ > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libavformat/avidec.c | 4 ++++ > libavformat/riff.c | 5 +++++ > libavformat/riff.h | 2 ++ > 3 files changed, 11 insertions(+) > > diff --git a/libavformat/avidec.c b/libavformat/avidec.c > index 1d887b1cc9..7946791fe4 100644 > --- a/libavformat/avidec.c > +++ b/libavformat/avidec.c > @@ -815,6 +815,10 @@ FF_ENABLE_DEPRECATION_WARNINGS > "mov tag found in avi (fourcc %s)\n", > av_fourcc2str(tag1)); > } > + /* Finally try unofficial codec tags to support non-standard files. */ The comment does not look useful. > + if (!st->codecpar->codec_id) > + st->codecpar->codec_id = ff_codec_get_id(ff_codec_bmp_tags_unofficial, tag1); > + > /* This is needed to get the pict type which is necessary > * for generating correct pts. */ > st->need_parsing = AVSTREAM_PARSE_HEADERS; > diff --git a/libavformat/riff.c b/libavformat/riff.c > index e755ad8d5f..52b0bf8f03 100644 > --- a/libavformat/riff.c > +++ b/libavformat/riff.c > @@ -491,6 +491,11 @@ const AVCodecTag ff_codec_bmp_tags[] = { > { AV_CODEC_ID_NONE, 0 } > }; > > +const AVCodecTag ff_codec_bmp_tags_unofficial[] = { > + { AV_CODEC_ID_HEVC, MKTAG('H', 'E', 'V', 'C') }, > + { AV_CODEC_ID_NONE, 0 } > +}; > + > const AVCodecTag ff_codec_wav_tags[] = { > { AV_CODEC_ID_PCM_S16LE, 0x0001 }, > /* must come after s16le in this list */ > diff --git a/libavformat/riff.h b/libavformat/riff.h > index 323aa38b4d..21078b77c8 100644 > --- a/libavformat/riff.h > +++ b/libavformat/riff.h > @@ -73,6 +73,8 @@ int ff_get_wav_header(AVFormatContext *s, AVIOContext *pb, AVCodecParameters *pa > extern const AVCodecTag ff_codec_bmp_tags[]; // exposed through avformat_get_riff_video_tags() > extern const AVCodecTag ff_codec_wav_tags[]; > > +extern const AVCodecTag ff_codec_bmp_tags_unofficial[]; Imo, this is far too complicated to avoid writing files that all other relevant applications write happily. Carl Eugen
On Sat, 24 Aug 2019, Carl Eugen Hoyos wrote: > Am Sa., 24. Aug. 2019 um 01:47 Uhr schrieb Marton Balint <cus@passwd.hu>: >> >> Some security cams generate this, as well as some versions of VirtualDub so >> support for _reading_ such files is justified. > > Please also mention vlc. Ok. > >> >> Fixes ticket #7110. >> >> See also this discussion: https://patchwork.ffmpeg.org/patch/8744/ >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> libavformat/avidec.c | 4 ++++ >> libavformat/riff.c | 5 +++++ >> libavformat/riff.h | 2 ++ >> 3 files changed, 11 insertions(+) >> >> diff --git a/libavformat/avidec.c b/libavformat/avidec.c >> index 1d887b1cc9..7946791fe4 100644 >> --- a/libavformat/avidec.c >> +++ b/libavformat/avidec.c >> @@ -815,6 +815,10 @@ FF_ENABLE_DEPRECATION_WARNINGS >> "mov tag found in avi (fourcc %s)\n", >> av_fourcc2str(tag1)); >> } > >> + /* Finally try unofficial codec tags to support non-standard files. */ > > The comment does not look useful. Ok, removed locally. > >> + if (!st->codecpar->codec_id) >> + st->codecpar->codec_id = ff_codec_get_id(ff_codec_bmp_tags_unofficial, tag1); >> + >> /* This is needed to get the pict type which is necessary >> * for generating correct pts. */ >> st->need_parsing = AVSTREAM_PARSE_HEADERS; >> diff --git a/libavformat/riff.c b/libavformat/riff.c >> index e755ad8d5f..52b0bf8f03 100644 >> --- a/libavformat/riff.c >> +++ b/libavformat/riff.c >> @@ -491,6 +491,11 @@ const AVCodecTag ff_codec_bmp_tags[] = { >> { AV_CODEC_ID_NONE, 0 } >> }; >> >> +const AVCodecTag ff_codec_bmp_tags_unofficial[] = { >> + { AV_CODEC_ID_HEVC, MKTAG('H', 'E', 'V', 'C') }, >> + { AV_CODEC_ID_NONE, 0 } >> +}; >> + >> const AVCodecTag ff_codec_wav_tags[] = { >> { AV_CODEC_ID_PCM_S16LE, 0x0001 }, >> /* must come after s16le in this list */ >> diff --git a/libavformat/riff.h b/libavformat/riff.h >> index 323aa38b4d..21078b77c8 100644 >> --- a/libavformat/riff.h >> +++ b/libavformat/riff.h >> @@ -73,6 +73,8 @@ int ff_get_wav_header(AVFormatContext *s, AVIOContext *pb, AVCodecParameters *pa >> extern const AVCodecTag ff_codec_bmp_tags[]; // exposed through avformat_get_riff_video_tags() >> extern const AVCodecTag ff_codec_wav_tags[]; >> >> +extern const AVCodecTag ff_codec_bmp_tags_unofficial[]; > > Imo, this is far too complicated to avoid writing files > that all other relevant applications write happily. I am personally OK if we simply add the tag to ff_codec_bmp_tags but there was an uproar against supporting writing such files. IMHO the cleanest way of doing things is allowing writing files _if_ std_complience is ALLOW_UNOFFICIAL, and otherwise rejecting it, but I am not intereseted in implementing this, but if you are then you can, and in order to separate the standard list and the unofficial list, you must have two... Regards, Marton
On Sat, Aug 24, 2019 at 5:48 AM Marton Balint <cus@passwd.hu> wrote: > > > On Sat, 24 Aug 2019, Carl Eugen Hoyos wrote: > > > Am Sa., 24. Aug. 2019 um 01:47 Uhr schrieb Marton Balint <cus@passwd.hu > >: > >> > >> Some security cams generate this, as well as some versions of > VirtualDub so > >> support for _reading_ such files is justified. > > > > Please also mention vlc. > > Ok. > > > > >> > >> Fixes ticket #7110. > >> > >> See also this discussion: https://patchwork.ffmpeg.org/patch/8744/ > >> > >> Signed-off-by: Marton Balint <cus@passwd.hu> > >> --- > >> libavformat/avidec.c | 4 ++++ > >> libavformat/riff.c | 5 +++++ > >> libavformat/riff.h | 2 ++ > >> 3 files changed, 11 insertions(+) > >> > >> diff --git a/libavformat/avidec.c b/libavformat/avidec.c > >> index 1d887b1cc9..7946791fe4 100644 > >> --- a/libavformat/avidec.c > >> +++ b/libavformat/avidec.c > >> @@ -815,6 +815,10 @@ FF_ENABLE_DEPRECATION_WARNINGS > >> "mov tag found in avi (fourcc %s)\n", > >> av_fourcc2str(tag1)); > >> } > > > >> + /* Finally try unofficial codec tags to support > non-standard files. */ > > > > The comment does not look useful. > > Ok, removed locally. > > > > >> + if (!st->codecpar->codec_id) > >> + st->codecpar->codec_id = > ff_codec_get_id(ff_codec_bmp_tags_unofficial, tag1); > >> + > >> /* This is needed to get the pict type which is > necessary > >> * for generating correct pts. */ > >> st->need_parsing = AVSTREAM_PARSE_HEADERS; > >> diff --git a/libavformat/riff.c b/libavformat/riff.c > >> index e755ad8d5f..52b0bf8f03 100644 > >> --- a/libavformat/riff.c > >> +++ b/libavformat/riff.c > >> @@ -491,6 +491,11 @@ const AVCodecTag ff_codec_bmp_tags[] = { > >> { AV_CODEC_ID_NONE, 0 } > >> }; > >> > >> +const AVCodecTag ff_codec_bmp_tags_unofficial[] = { > >> + { AV_CODEC_ID_HEVC, MKTAG('H', 'E', 'V', 'C') }, > >> + { AV_CODEC_ID_NONE, 0 } > >> +}; > >> + > >> const AVCodecTag ff_codec_wav_tags[] = { > >> { AV_CODEC_ID_PCM_S16LE, 0x0001 }, > >> /* must come after s16le in this list */ > >> diff --git a/libavformat/riff.h b/libavformat/riff.h > >> index 323aa38b4d..21078b77c8 100644 > >> --- a/libavformat/riff.h > >> +++ b/libavformat/riff.h > >> @@ -73,6 +73,8 @@ int ff_get_wav_header(AVFormatContext *s, AVIOContext > *pb, AVCodecParameters *pa > >> extern const AVCodecTag ff_codec_bmp_tags[]; // exposed through > avformat_get_riff_video_tags() > >> extern const AVCodecTag ff_codec_wav_tags[]; > >> > >> +extern const AVCodecTag ff_codec_bmp_tags_unofficial[]; > > > > Imo, this is far too complicated to avoid writing files > > that all other relevant applications write happily. > > I am personally OK if we simply add the tag to ff_codec_bmp_tags but there > was an uproar against supporting writing such files. > > IMHO the cleanest way of doing things is allowing writing files _if_ > std_complience is ALLOW_UNOFFICIAL, and otherwise rejecting it, but I am > not intereseted in implementing this, but if you are then you can, and in > order to separate the standard list and the unofficial list, you must have > two... > > Regards, > Marton > That was my suggestion as well. It's what we do in movenc when dealing with codecs that have non-finished encapsulation specs (VP9/AV1 at some point, FLAC currently, etc).
On Sat, 24 Aug 2019, James Almer wrote: > On Sat, Aug 24, 2019 at 5:48 AM Marton Balint <cus@passwd.hu> wrote: > >> >> >> On Sat, 24 Aug 2019, Carl Eugen Hoyos wrote: >> >> > Am Sa., 24. Aug. 2019 um 01:47 Uhr schrieb Marton Balint <cus@passwd.hu >> >: >> >> >> >> Some security cams generate this, as well as some versions of >> VirtualDub so >> >> support for _reading_ such files is justified. >> > >> > Please also mention vlc. >> >> Ok. >> >> > >> >> >> >> Fixes ticket #7110. >> >> >> >> See also this discussion: https://patchwork.ffmpeg.org/patch/8744/ >> >> >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> >> --- >> >> libavformat/avidec.c | 4 ++++ >> >> libavformat/riff.c | 5 +++++ >> >> libavformat/riff.h | 2 ++ >> >> 3 files changed, 11 insertions(+) >> >> >> >> diff --git a/libavformat/avidec.c b/libavformat/avidec.c >> >> index 1d887b1cc9..7946791fe4 100644 >> >> --- a/libavformat/avidec.c >> >> +++ b/libavformat/avidec.c >> >> @@ -815,6 +815,10 @@ FF_ENABLE_DEPRECATION_WARNINGS >> >> "mov tag found in avi (fourcc %s)\n", >> >> av_fourcc2str(tag1)); >> >> } >> > >> >> + /* Finally try unofficial codec tags to support >> non-standard files. */ >> > >> > The comment does not look useful. >> >> Ok, removed locally. >> >> > >> >> + if (!st->codecpar->codec_id) >> >> + st->codecpar->codec_id = >> ff_codec_get_id(ff_codec_bmp_tags_unofficial, tag1); >> >> + >> >> /* This is needed to get the pict type which is >> necessary >> >> * for generating correct pts. */ >> >> st->need_parsing = AVSTREAM_PARSE_HEADERS; >> >> diff --git a/libavformat/riff.c b/libavformat/riff.c >> >> index e755ad8d5f..52b0bf8f03 100644 >> >> --- a/libavformat/riff.c >> >> +++ b/libavformat/riff.c >> >> @@ -491,6 +491,11 @@ const AVCodecTag ff_codec_bmp_tags[] = { >> >> { AV_CODEC_ID_NONE, 0 } >> >> }; >> >> >> >> +const AVCodecTag ff_codec_bmp_tags_unofficial[] = { >> >> + { AV_CODEC_ID_HEVC, MKTAG('H', 'E', 'V', 'C') }, >> >> + { AV_CODEC_ID_NONE, 0 } >> >> +}; >> >> + >> >> const AVCodecTag ff_codec_wav_tags[] = { >> >> { AV_CODEC_ID_PCM_S16LE, 0x0001 }, >> >> /* must come after s16le in this list */ >> >> diff --git a/libavformat/riff.h b/libavformat/riff.h >> >> index 323aa38b4d..21078b77c8 100644 >> >> --- a/libavformat/riff.h >> >> +++ b/libavformat/riff.h >> >> @@ -73,6 +73,8 @@ int ff_get_wav_header(AVFormatContext *s, AVIOContext >> *pb, AVCodecParameters *pa >> >> extern const AVCodecTag ff_codec_bmp_tags[]; // exposed through >> avformat_get_riff_video_tags() >> >> extern const AVCodecTag ff_codec_wav_tags[]; >> >> >> >> +extern const AVCodecTag ff_codec_bmp_tags_unofficial[]; >> > >> > Imo, this is far too complicated to avoid writing files >> > that all other relevant applications write happily. >> >> I am personally OK if we simply add the tag to ff_codec_bmp_tags but there >> was an uproar against supporting writing such files. >> >> IMHO the cleanest way of doing things is allowing writing files _if_ >> std_complience is ALLOW_UNOFFICIAL, and otherwise rejecting it, but I am >> not intereseted in implementing this, but if you are then you can, and in >> order to separate the standard list and the unofficial list, you must have >> two... >> >> Regards, >> Marton >> > > That was my suggestion as well. It's what we do in movenc when dealing with > codecs that have non-finished encapsulation specs (VP9/AV1 at some point, > FLAC currently, etc). Will apply soon. Regards, Marton
On Thu, 29 Aug 2019, Marton Balint wrote: > > > On Sat, 24 Aug 2019, James Almer wrote: > >> On Sat, Aug 24, 2019 at 5:48 AM Marton Balint <cus@passwd.hu> wrote: >> >>> >>> >>> On Sat, 24 Aug 2019, Carl Eugen Hoyos wrote: >>> >>> > Am Sa., 24. Aug. 2019 um 01:47 Uhr schrieb Marton Balint <cus@passwd.hu >>> >: >>> >> >>> >> Some security cams generate this, as well as some versions of >>> VirtualDub so >>> >> support for _reading_ such files is justified. >>> > >>> > Please also mention vlc. >>> >>> Ok. >>> >>> > >>> >> >>> >> Fixes ticket #7110. >>> >> >>> >> See also this discussion: https://patchwork.ffmpeg.org/patch/8744/ >>> >> >>> >> Signed-off-by: Marton Balint <cus@passwd.hu> >>> >> --- >>> >> libavformat/avidec.c | 4 ++++ >>> >> libavformat/riff.c | 5 +++++ >>> >> libavformat/riff.h | 2 ++ >>> >> 3 files changed, 11 insertions(+) >>> >> >>> >> diff --git a/libavformat/avidec.c b/libavformat/avidec.c >>> >> index 1d887b1cc9..7946791fe4 100644 >>> >> --- a/libavformat/avidec.c >>> >> +++ b/libavformat/avidec.c >>> >> @@ -815,6 +815,10 @@ FF_ENABLE_DEPRECATION_WARNINGS >>> >> "mov tag found in avi (fourcc > %s)\n", >>> >> av_fourcc2str(tag1)); >>> >> } >>> > >>> >> + /* Finally try unofficial codec tags to support >>> non-standard files. */ >>> > >>> > The comment does not look useful. >>> >>> Ok, removed locally. >>> >>> > >>> >> + if (!st->codecpar->codec_id) >>> >> + st->codecpar->codec_id = >>> ff_codec_get_id(ff_codec_bmp_tags_unofficial, tag1); >>> >> + >>> >> /* This is needed to get the pict type which is >>> necessary >>> >> * for generating correct pts. */ >>> >> st->need_parsing = AVSTREAM_PARSE_HEADERS; >>> >> diff --git a/libavformat/riff.c b/libavformat/riff.c >>> >> index e755ad8d5f..52b0bf8f03 100644 >>> >> --- a/libavformat/riff.c >>> >> +++ b/libavformat/riff.c >>> >> @@ -491,6 +491,11 @@ const AVCodecTag ff_codec_bmp_tags[] = { >>> >> { AV_CODEC_ID_NONE, 0 } >>> >> }; >>> >> >>> >> +const AVCodecTag ff_codec_bmp_tags_unofficial[] = { >>> >> + { AV_CODEC_ID_HEVC, MKTAG('H', 'E', 'V', 'C') }, >>> >> + { AV_CODEC_ID_NONE, 0 } >>> >> +}; >>> >> + >>> >> const AVCodecTag ff_codec_wav_tags[] = { >>> >> { AV_CODEC_ID_PCM_S16LE, 0x0001 }, >>> >> /* must come after s16le in this list */ >>> >> diff --git a/libavformat/riff.h b/libavformat/riff.h >>> >> index 323aa38b4d..21078b77c8 100644 >>> >> --- a/libavformat/riff.h >>> >> +++ b/libavformat/riff.h >>> >> @@ -73,6 +73,8 @@ int ff_get_wav_header(AVFormatContext *s, AVIOContext >>> *pb, AVCodecParameters *pa >>> >> extern const AVCodecTag ff_codec_bmp_tags[]; // exposed through >>> avformat_get_riff_video_tags() >>> >> extern const AVCodecTag ff_codec_wav_tags[]; >>> >> >>> >> +extern const AVCodecTag ff_codec_bmp_tags_unofficial[]; >>> > >>> > Imo, this is far too complicated to avoid writing files >>> > that all other relevant applications write happily. >>> >>> I am personally OK if we simply add the tag to ff_codec_bmp_tags but there >>> was an uproar against supporting writing such files. >>> >>> IMHO the cleanest way of doing things is allowing writing files _if_ >>> std_complience is ALLOW_UNOFFICIAL, and otherwise rejecting it, but I am >>> not intereseted in implementing this, but if you are then you can, and in >>> order to separate the standard list and the unofficial list, you must have >>> two... >>> >>> Regards, >>> Marton >>> >> >> That was my suggestion as well. It's what we do in movenc when dealing with >> codecs that have non-finished encapsulation specs (VP9/AV1 at some point, >> FLAC currently, etc). > > Will apply soon. Applied. Regards, Marton
diff --git a/libavformat/avidec.c b/libavformat/avidec.c index 1d887b1cc9..7946791fe4 100644 --- a/libavformat/avidec.c +++ b/libavformat/avidec.c @@ -815,6 +815,10 @@ FF_ENABLE_DEPRECATION_WARNINGS "mov tag found in avi (fourcc %s)\n", av_fourcc2str(tag1)); } + /* Finally try unofficial codec tags to support non-standard files. */ + if (!st->codecpar->codec_id) + st->codecpar->codec_id = ff_codec_get_id(ff_codec_bmp_tags_unofficial, tag1); + /* This is needed to get the pict type which is necessary * for generating correct pts. */ st->need_parsing = AVSTREAM_PARSE_HEADERS; diff --git a/libavformat/riff.c b/libavformat/riff.c index e755ad8d5f..52b0bf8f03 100644 --- a/libavformat/riff.c +++ b/libavformat/riff.c @@ -491,6 +491,11 @@ const AVCodecTag ff_codec_bmp_tags[] = { { AV_CODEC_ID_NONE, 0 } }; +const AVCodecTag ff_codec_bmp_tags_unofficial[] = { + { AV_CODEC_ID_HEVC, MKTAG('H', 'E', 'V', 'C') }, + { AV_CODEC_ID_NONE, 0 } +}; + const AVCodecTag ff_codec_wav_tags[] = { { AV_CODEC_ID_PCM_S16LE, 0x0001 }, /* must come after s16le in this list */ diff --git a/libavformat/riff.h b/libavformat/riff.h index 323aa38b4d..21078b77c8 100644 --- a/libavformat/riff.h +++ b/libavformat/riff.h @@ -73,6 +73,8 @@ int ff_get_wav_header(AVFormatContext *s, AVIOContext *pb, AVCodecParameters *pa extern const AVCodecTag ff_codec_bmp_tags[]; // exposed through avformat_get_riff_video_tags() extern const AVCodecTag ff_codec_wav_tags[]; +extern const AVCodecTag ff_codec_bmp_tags_unofficial[]; + void ff_parse_specific_params(AVStream *st, int *au_rate, int *au_ssize, int *au_scale); int ff_read_riff_info(AVFormatContext *s, int64_t size);
Some security cams generate this, as well as some versions of VirtualDub so support for _reading_ such files is justified. Fixes ticket #7110. See also this discussion: https://patchwork.ffmpeg.org/patch/8744/ Signed-off-by: Marton Balint <cus@passwd.hu> --- libavformat/avidec.c | 4 ++++ libavformat/riff.c | 5 +++++ libavformat/riff.h | 2 ++ 3 files changed, 11 insertions(+)