| Message ID | 20181001180146.49151-1-alx.sukhanov@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Oct 1, 2018 at 11:01 AM <alx.sukhanov@gmail.com> wrote: > From: Alex Sukhanov <asukhanov@google.com> > > --- > libavformat/ivfenc.c | 50 +++++++++++++++++++++++++++++++++----------- > 1 file changed, 38 insertions(+), 12 deletions(-) > > diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c > index 66441a2a43..6410828533 100644 > --- a/libavformat/ivfenc.c > +++ b/libavformat/ivfenc.c > @@ -36,19 +36,29 @@ static int ivf_write_header(AVFormatContext *s) > return AVERROR(EINVAL); > } > par = s->streams[0]->codecpar; > - if (par->codec_type != AVMEDIA_TYPE_VIDEO || > - !(par->codec_id == AV_CODEC_ID_AV1 || > - par->codec_id == AV_CODEC_ID_VP8 || > - par->codec_id == AV_CODEC_ID_VP9)) { > - av_log(s, AV_LOG_ERROR, "Currently only VP8, VP9 and AV1 are > supported!\n"); > - return AVERROR(EINVAL); > - } > avio_write(pb, "DKIF", 4); > avio_wl16(pb, 0); // version > avio_wl16(pb, 32); // header length > - avio_wl32(pb, > - par->codec_id == AV_CODEC_ID_VP9 ? AV_RL32("VP90") : > - par->codec_id == AV_CODEC_ID_VP8 ? AV_RL32("VP80") : > AV_RL32("AV01")); > + switch (par->codec_id) { > + case AV_CODEC_ID_AV1: > + avio_wl32(pb, AV_RL32("AV01")); > + break; > + case AV_CODEC_ID_H264: > + avio_wl32(pb, AV_RL32("H264")); > + break; > + case AV_CODEC_ID_HEVC: > + avio_wl32(pb, AV_RL32("HEVC")); > + break; > + case AV_CODEC_ID_VP8: > + avio_wl32(pb, AV_RL32("VP80")); > + break; > + case AV_CODEC_ID_VP9: > + avio_wl32(pb, AV_RL32("VP90")); > + break; > + default: > + av_log(s, AV_LOG_ERROR, "Currently only AV1, H264, HEVC, VP8 and > VP9 and AV1 are supported!\n"); > + return AVERROR(EINVAL); > + } > avio_wl16(pb, par->width); > avio_wl16(pb, par->height); > avio_wl32(pb, s->streams[0]->time_base.den); > @@ -95,16 +105,32 @@ static int ivf_check_bitstream(struct AVFormatContext > *s, const AVPacket *pkt) > int ret = 1; > AVStream *st = s->streams[pkt->stream_index]; > > - if (st->codecpar->codec_id == AV_CODEC_ID_VP9) > + if (st->codecpar->codec_id == AV_CODEC_ID_H264) { > + if (pkt->size >= 5 && AV_RB32(pkt->data) != 0x0000001 && > + (AV_RB24(pkt->data) != 0x000001 || > + (st->codecpar->extradata_size > 0 && > + st->codecpar->extradata[0] == 1))) > + ret = ff_stream_add_bitstream_filter(st, "h264_mp4toannexb", > NULL); > + } else if (st->codecpar->codec_id == AV_CODEC_ID_HEVC) { > + if (pkt->size >= 5 && AV_RB32(pkt->data) != 0x0000001 && > + (AV_RB24(pkt->data) != 0x000001 || > + (st->codecpar->extradata_size > 0 && > + st->codecpar->extradata[0] == 1))) > + ret = ff_stream_add_bitstream_filter(st, "hevc_mp4toannexb", > NULL); > + } else if (st->codecpar->codec_id == AV_CODEC_ID_VP9) { > ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", NULL); > + } > > return ret; > } > > static const AVCodecTag codec_ivf_tags[] = { > + { AV_CODEC_ID_AV1, MKTAG('A', 'V', '0', '1') }, > + { AV_CODEC_ID_H264, MKTAG('H', '2', '6', '4') }, > + { AV_CODEC_ID_HEVC, MKTAG('H', 'E', 'V', 'C') }, > { AV_CODEC_ID_VP8, MKTAG('V', 'P', '8', '0') }, > { AV_CODEC_ID_VP9, MKTAG('V', 'P', '9', '0') }, > - { AV_CODEC_ID_AV1, MKTAG('A', 'V', '0', '1') }, > + > { AV_CODEC_ID_NONE, 0 } > }; > > -- > 2.19.0.605.g01d371f741-goog > > Can you please take a look on this patch? Thank you
On 05/10/18 21:45, Alex Sukhanov wrote: > On Mon, Oct 1, 2018 at 11:01 AM <alx.sukhanov@gmail.com> wrote: > >> From: Alex Sukhanov <asukhanov@google.com> >> >> --- >> libavformat/ivfenc.c | 50 +++++++++++++++++++++++++++++++++----------- >> 1 file changed, 38 insertions(+), 12 deletions(-) >> >> diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c >> index 66441a2a43..6410828533 100644 >> --- a/libavformat/ivfenc.c >> +++ b/libavformat/ivfenc.c >> @@ -36,19 +36,29 @@ static int ivf_write_header(AVFormatContext *s) >> return AVERROR(EINVAL); >> } >> par = s->streams[0]->codecpar; >> - if (par->codec_type != AVMEDIA_TYPE_VIDEO || >> - !(par->codec_id == AV_CODEC_ID_AV1 || >> - par->codec_id == AV_CODEC_ID_VP8 || >> - par->codec_id == AV_CODEC_ID_VP9)) { >> - av_log(s, AV_LOG_ERROR, "Currently only VP8, VP9 and AV1 are >> supported!\n"); >> - return AVERROR(EINVAL); >> - } >> avio_write(pb, "DKIF", 4); >> avio_wl16(pb, 0); // version >> avio_wl16(pb, 32); // header length >> - avio_wl32(pb, >> - par->codec_id == AV_CODEC_ID_VP9 ? AV_RL32("VP90") : >> - par->codec_id == AV_CODEC_ID_VP8 ? AV_RL32("VP80") : >> AV_RL32("AV01")); >> + switch (par->codec_id) { >> + case AV_CODEC_ID_AV1: >> + avio_wl32(pb, AV_RL32("AV01")); >> + break; >> + case AV_CODEC_ID_H264: >> + avio_wl32(pb, AV_RL32("H264")); >> + break; >> + case AV_CODEC_ID_HEVC: >> + avio_wl32(pb, AV_RL32("HEVC")); >> + break; >> + case AV_CODEC_ID_VP8: >> + avio_wl32(pb, AV_RL32("VP80")); >> + break; >> + case AV_CODEC_ID_VP9: >> + avio_wl32(pb, AV_RL32("VP90")); >> + break; >> + default: >> + av_log(s, AV_LOG_ERROR, "Currently only AV1, H264, HEVC, VP8 and >> VP9 and AV1 are supported!\n"); >> + return AVERROR(EINVAL); >> + } >> avio_wl16(pb, par->width); >> avio_wl16(pb, par->height); >> avio_wl32(pb, s->streams[0]->time_base.den); >> @@ -95,16 +105,32 @@ static int ivf_check_bitstream(struct AVFormatContext >> *s, const AVPacket *pkt) >> int ret = 1; >> AVStream *st = s->streams[pkt->stream_index]; >> >> - if (st->codecpar->codec_id == AV_CODEC_ID_VP9) >> + if (st->codecpar->codec_id == AV_CODEC_ID_H264) { >> + if (pkt->size >= 5 && AV_RB32(pkt->data) != 0x0000001 && >> + (AV_RB24(pkt->data) != 0x000001 || >> + (st->codecpar->extradata_size > 0 && >> + st->codecpar->extradata[0] == 1))) >> + ret = ff_stream_add_bitstream_filter(st, "h264_mp4toannexb", >> NULL); >> + } else if (st->codecpar->codec_id == AV_CODEC_ID_HEVC) { >> + if (pkt->size >= 5 && AV_RB32(pkt->data) != 0x0000001 && >> + (AV_RB24(pkt->data) != 0x000001 || >> + (st->codecpar->extradata_size > 0 && >> + st->codecpar->extradata[0] == 1))) >> + ret = ff_stream_add_bitstream_filter(st, "hevc_mp4toannexb", >> NULL); >> + } else if (st->codecpar->codec_id == AV_CODEC_ID_VP9) { >> ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", NULL); >> + } >> >> return ret; >> } >> >> static const AVCodecTag codec_ivf_tags[] = { >> + { AV_CODEC_ID_AV1, MKTAG('A', 'V', '0', '1') }, >> + { AV_CODEC_ID_H264, MKTAG('H', '2', '6', '4') }, >> + { AV_CODEC_ID_HEVC, MKTAG('H', 'E', 'V', 'C') }, >> { AV_CODEC_ID_VP8, MKTAG('V', 'P', '8', '0') }, >> { AV_CODEC_ID_VP9, MKTAG('V', 'P', '9', '0') }, >> - { AV_CODEC_ID_AV1, MKTAG('A', 'V', '0', '1') }, >> + >> { AV_CODEC_ID_NONE, 0 } >> }; >> >> -- >> 2.19.0.605.g01d371f741-goog >> >> > Can you please take a look on this patch? Can you answer the question posed in <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-September/234655.html> about the reasoning for this patch? In particular, it might be helpful if you could point out any tools/standards which support (or are in future intending to support) this format - in my experience pretty much everything dealing with H.26[45] raw streams uses the Annex B format. Thanks, - Mark
On Sat, Oct 6, 2018 at 10:37 AM Mark Thompson <sw@jkqxz.net> wrote: > On 05/10/18 21:45, Alex Sukhanov wrote: > > On Mon, Oct 1, 2018 at 11:01 AM <alx.sukhanov@gmail.com> wrote: > > > >> From: Alex Sukhanov <asukhanov@google.com> > >> > >> --- > >> libavformat/ivfenc.c | 50 +++++++++++++++++++++++++++++++++----------- > >> 1 file changed, 38 insertions(+), 12 deletions(-) > >> > >> diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c > >> index 66441a2a43..6410828533 100644 > >> --- a/libavformat/ivfenc.c > >> +++ b/libavformat/ivfenc.c > >> @@ -36,19 +36,29 @@ static int ivf_write_header(AVFormatContext *s) > >> return AVERROR(EINVAL); > >> } > >> par = s->streams[0]->codecpar; > >> - if (par->codec_type != AVMEDIA_TYPE_VIDEO || > >> - !(par->codec_id == AV_CODEC_ID_AV1 || > >> - par->codec_id == AV_CODEC_ID_VP8 || > >> - par->codec_id == AV_CODEC_ID_VP9)) { > >> - av_log(s, AV_LOG_ERROR, "Currently only VP8, VP9 and AV1 are > >> supported!\n"); > >> - return AVERROR(EINVAL); > >> - } > >> avio_write(pb, "DKIF", 4); > >> avio_wl16(pb, 0); // version > >> avio_wl16(pb, 32); // header length > >> - avio_wl32(pb, > >> - par->codec_id == AV_CODEC_ID_VP9 ? AV_RL32("VP90") : > >> - par->codec_id == AV_CODEC_ID_VP8 ? AV_RL32("VP80") : > >> AV_RL32("AV01")); > >> + switch (par->codec_id) { > >> + case AV_CODEC_ID_AV1: > >> + avio_wl32(pb, AV_RL32("AV01")); > >> + break; > >> + case AV_CODEC_ID_H264: > >> + avio_wl32(pb, AV_RL32("H264")); > >> + break; > >> + case AV_CODEC_ID_HEVC: > >> + avio_wl32(pb, AV_RL32("HEVC")); > >> + break; > >> + case AV_CODEC_ID_VP8: > >> + avio_wl32(pb, AV_RL32("VP80")); > >> + break; > >> + case AV_CODEC_ID_VP9: > >> + avio_wl32(pb, AV_RL32("VP90")); > >> + break; > >> + default: > >> + av_log(s, AV_LOG_ERROR, "Currently only AV1, H264, HEVC, VP8 > and > >> VP9 and AV1 are supported!\n"); > >> + return AVERROR(EINVAL); > >> + } > >> avio_wl16(pb, par->width); > >> avio_wl16(pb, par->height); > >> avio_wl32(pb, s->streams[0]->time_base.den); > >> @@ -95,16 +105,32 @@ static int ivf_check_bitstream(struct > AVFormatContext > >> *s, const AVPacket *pkt) > >> int ret = 1; > >> AVStream *st = s->streams[pkt->stream_index]; > >> > >> - if (st->codecpar->codec_id == AV_CODEC_ID_VP9) > >> + if (st->codecpar->codec_id == AV_CODEC_ID_H264) { > >> + if (pkt->size >= 5 && AV_RB32(pkt->data) != 0x0000001 && > >> + (AV_RB24(pkt->data) != 0x000001 || > >> + (st->codecpar->extradata_size > 0 && > >> + st->codecpar->extradata[0] == 1))) > >> + ret = ff_stream_add_bitstream_filter(st, > "h264_mp4toannexb", > >> NULL); > >> + } else if (st->codecpar->codec_id == AV_CODEC_ID_HEVC) { > >> + if (pkt->size >= 5 && AV_RB32(pkt->data) != 0x0000001 && > >> + (AV_RB24(pkt->data) != 0x000001 || > >> + (st->codecpar->extradata_size > 0 && > >> + st->codecpar->extradata[0] == 1))) > >> + ret = ff_stream_add_bitstream_filter(st, > "hevc_mp4toannexb", > >> NULL); > >> + } else if (st->codecpar->codec_id == AV_CODEC_ID_VP9) { > >> ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", > NULL); > >> + } > >> > >> return ret; > >> } > >> > >> static const AVCodecTag codec_ivf_tags[] = { > >> + { AV_CODEC_ID_AV1, MKTAG('A', 'V', '0', '1') }, > >> + { AV_CODEC_ID_H264, MKTAG('H', '2', '6', '4') }, > >> + { AV_CODEC_ID_HEVC, MKTAG('H', 'E', 'V', 'C') }, > >> { AV_CODEC_ID_VP8, MKTAG('V', 'P', '8', '0') }, > >> { AV_CODEC_ID_VP9, MKTAG('V', 'P', '9', '0') }, > >> - { AV_CODEC_ID_AV1, MKTAG('A', 'V', '0', '1') }, > >> + > >> { AV_CODEC_ID_NONE, 0 } > >> }; > >> > >> -- > >> 2.19.0.605.g01d371f741-goog > >> > >> > > Can you please take a look on this patch? > > Can you answer the question posed in < > https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-September/234655.html> > about the reasoning for this patch? > > In particular, it might be helpful if you could point out any > tools/standards which support (or are in future intending to support) this > format - in my experience pretty much everything dealing with H.26[45] raw > streams uses the Annex B format. > > Thanks, > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Hi Mark, at Google we have some old service which is still running and it works only with the IVF container. It would be great if ffmpeg could generate such videos so we could give them to the service then. Given that ffmpeg IVF demuxer already supports reading such files, I think it's reasonable to make IVF muxer be able to generate them. Hope it answers the question. Thank you
On Thu, Oct 11, 2018 at 10:58 PM Alex Sukhanov <alx.sukhanov@gmail.com> wrote: > > Hi Mark, > > at Google we have some old service which is still running and it works only > with the IVF container. It would be great if ffmpeg could generate such > videos so we could give them to the service then. Given that ffmpeg IVF > demuxer already supports reading such files, I think it's reasonable to > make IVF muxer be able to generate them. > Hope it answers the question. > > Thank you Given the amount of code is not large I'm not against having it in, but if it's not something that ever was meant to go into the public I'd probably disable creation of such files unless you enable a less standards-compliant strictness mode. Or if others are against it, I'm OK with this thing not being included as it clearly is to enable some GOOG internal workflows only. Best regards, Jan
On 11/10/2018 22:21, Jan Ekström wrote: > I'd probably disable creation of such files unless you enable a less > standards-compliant strictness mode. Is there even an IVF spec, though? If not, there's not really such a thing as "standards-compliant". - Derek
On Thu, Oct 11, 2018 at 2:47 PM Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote: > On 11/10/2018 22:21, Jan Ekström wrote: > > I'd probably disable creation of such files unless you enable a less > > standards-compliant strictness mode. > > Is there even an IVF spec, though? If not, there's not really such a > thing as "standards-compliant". > > - Derek > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel The only "spec" I'm aware of: https://wiki.multimedia.cx/index.php/IVF
On 11/10/2018 23:39, Alex Sukhanov wrote:
> The only "spec" I'm aware of:https://wiki.multimedia.cx/index.php/IVF
Yeah, not really a spec.
I have no strong objections, I guess.
- Derek
On Fri, Oct 12, 2018 at 01:13:45AM +0100, Derek Buitenhuis wrote: > On 11/10/2018 23:39, Alex Sukhanov wrote: > > The only "spec" I'm aware of:https://wiki.multimedia.cx/index.php/IVF > > Yeah, not really a spec. > > I have no strong objections, I guess. so IIUC noone is against this ? if so i will apply the last patch in a few days unless i forget or someone else does before thx [...]
On Sun, Oct 14, 2018 at 3:17 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Fri, Oct 12, 2018 at 01:13:45AM +0100, Derek Buitenhuis wrote: > > On 11/10/2018 23:39, Alex Sukhanov wrote: > > > The only "spec" I'm aware of:https://wiki.multimedia.cx/index.php/IVF > > > > Yeah, not really a spec. > > > > I have no strong objections, I guess. > > so IIUC noone is against this ? > if so i will apply the last patch in a few days unless i forget or > someone else does before > > thx > As mentioned, since this is nothing should have ever been used outside of one of GOOG's legacy systems, I would only apply this with a warning and strictness requirement of experimental or whatever level matches these sorts of cases. Nothing in public that utilizes IVF files will output or read AVC/HEVC from IVF files. It was explicitly only utilized by the webm/libvpx project publicly, and that for obvious reasons will not support AVC/HEVC. Best regards, Jan
On Sun, Oct 14, 2018 at 1:27 PM Jan Ekström <jeebjp@gmail.com> wrote: > > On Sun, Oct 14, 2018 at 3:17 AM Michael Niedermayer > <michael@niedermayer.cc> wrote: > > > > On Fri, Oct 12, 2018 at 01:13:45AM +0100, Derek Buitenhuis wrote: > > > On 11/10/2018 23:39, Alex Sukhanov wrote: > > > > The only "spec" I'm aware of:https://wiki.multimedia.cx/index.php/IVF > > > > > > Yeah, not really a spec. > > > > > > I have no strong objections, I guess. > > > > so IIUC noone is against this ? > > if so i will apply the last patch in a few days unless i forget or > > someone else does before > > > > thx > > > > As mentioned, since this is nothing should have ever been used outside > of one of GOOG's legacy systems, I would only apply this with a > warning and strictness requirement of experimental or whatever level > matches these sorts of cases. > For the record, this was regarding the muxer so that unknowing people will not generate such files that nothing else can read. Best regards, Jan
On Sun, Oct 14, 2018 at 01:29:53PM +0300, Jan Ekström wrote: > On Sun, Oct 14, 2018 at 1:27 PM Jan Ekström <jeebjp@gmail.com> wrote: > > > > On Sun, Oct 14, 2018 at 3:17 AM Michael Niedermayer > > <michael@niedermayer.cc> wrote: > > > > > > On Fri, Oct 12, 2018 at 01:13:45AM +0100, Derek Buitenhuis wrote: > > > > On 11/10/2018 23:39, Alex Sukhanov wrote: > > > > > The only "spec" I'm aware of:https://wiki.multimedia.cx/index.php/IVF > > > > > > > > Yeah, not really a spec. > > > > > > > > I have no strong objections, I guess. > > > > > > so IIUC noone is against this ? > > > if so i will apply the last patch in a few days unless i forget or > > > someone else does before > > > > > > thx > > > > > > > As mentioned, since this is nothing should have ever been used outside > > of one of GOOG's legacy systems, I would only apply this with a > > warning and strictness requirement of experimental or whatever level > > matches these sorts of cases. > > > > For the record, this was regarding the muxer so that unknowing people > will not generate such files that nothing else can read. If you choose a random container and store a random codec in it (which you can do) chances are theres not much that will play it. for example i just tried muxing a stream of png images into mpeg and FFmpeg does that without complaining. ./ffmpeg -i matrixbench_mpeg2.mpg -vcodec png test.mpeg So if we do not check this for a major format that has a proper specification which i belive nowhere allows png Why should we add a limitation on a format that has nothing saying that you cannot put some other codec into it ? And i mean there IS software that uses that what is asked for here. If you object because the text at https://wiki.multimedia.cx/index.php/IVF does not list all codecs. Well its a wiki, the author of the patch could get an account and update this. In fact independant of this discussion here it surely wont hurt if the people using the format review and update that page. The other concern IIUC (please correct me if i misunderstood) is that its a legacy format. FFmpeg is full of legacy format support, we suport some really bizare formats ... And another concern IIUC ( again please correct me if i misunderstood) is that someone might unintentionally generate a unsupported file this is tricky, the default video codec for ivf is AV_CODEC_ID_VP8 to get something else, one would need to manually override this and as we already saw thats not something we current protect against very well. Iam not against disallowing the new codecs by default but it feels inconsistent compared to other containers. what do you say ? did i convince you or should we disallow it and add a check. Iam happy with whatever people agree on thanks [...]
On 10/14/2018 9:59 AM, Michael Niedermayer wrote: > On Sun, Oct 14, 2018 at 01:29:53PM +0300, Jan Ekström wrote: >> On Sun, Oct 14, 2018 at 1:27 PM Jan Ekström <jeebjp@gmail.com> wrote: >>> >>> On Sun, Oct 14, 2018 at 3:17 AM Michael Niedermayer >>> <michael@niedermayer.cc> wrote: >>>> >>>> On Fri, Oct 12, 2018 at 01:13:45AM +0100, Derek Buitenhuis wrote: >>>>> On 11/10/2018 23:39, Alex Sukhanov wrote: >>>>>> The only "spec" I'm aware of:https://wiki.multimedia.cx/index.php/IVF >>>>> >>>>> Yeah, not really a spec. >>>>> >>>>> I have no strong objections, I guess. >>>> >>>> so IIUC noone is against this ? >>>> if so i will apply the last patch in a few days unless i forget or >>>> someone else does before >>>> >>>> thx >>>> >>> >>> As mentioned, since this is nothing should have ever been used outside >>> of one of GOOG's legacy systems, I would only apply this with a >>> warning and strictness requirement of experimental or whatever level >>> matches these sorts of cases. >>> >> >> For the record, this was regarding the muxer so that unknowing people >> will not generate such files that nothing else can read. > > If you choose a random container and store a random codec in it (which you can do) > chances are theres not much that will play it. > > for example i just tried muxing a stream of png images into mpeg and FFmpeg > does that without complaining. > ./ffmpeg -i matrixbench_mpeg2.mpg -vcodec png test.mpeg > > So if we do not check this for a major format that has a proper specification > which i belive nowhere allows png > > Why should we add a limitation on a format that has nothing saying that you > cannot put some other codec into it ? The proper thing to do would be to effectively disallow such invalid muxing scenarios, fixing this instead of adding even more wrong cases to the pile. I'm fairly sure we blocked a patch to allow muxing hevc into flv years ago. That's what needs to be done in general.
On Sun, Oct 14, 2018 at 10:24:22AM -0300, James Almer wrote: > On 10/14/2018 9:59 AM, Michael Niedermayer wrote: > > On Sun, Oct 14, 2018 at 01:29:53PM +0300, Jan Ekström wrote: > >> On Sun, Oct 14, 2018 at 1:27 PM Jan Ekström <jeebjp@gmail.com> wrote: > >>> > >>> On Sun, Oct 14, 2018 at 3:17 AM Michael Niedermayer > >>> <michael@niedermayer.cc> wrote: > >>>> > >>>> On Fri, Oct 12, 2018 at 01:13:45AM +0100, Derek Buitenhuis wrote: > >>>>> On 11/10/2018 23:39, Alex Sukhanov wrote: > >>>>>> The only "spec" I'm aware of:https://wiki.multimedia.cx/index.php/IVF > >>>>> > >>>>> Yeah, not really a spec. > >>>>> > >>>>> I have no strong objections, I guess. > >>>> > >>>> so IIUC noone is against this ? > >>>> if so i will apply the last patch in a few days unless i forget or > >>>> someone else does before > >>>> > >>>> thx > >>>> > >>> > >>> As mentioned, since this is nothing should have ever been used outside > >>> of one of GOOG's legacy systems, I would only apply this with a > >>> warning and strictness requirement of experimental or whatever level > >>> matches these sorts of cases. > >>> > >> > >> For the record, this was regarding the muxer so that unknowing people > >> will not generate such files that nothing else can read. > > > > If you choose a random container and store a random codec in it (which you can do) > > chances are theres not much that will play it. > > > > for example i just tried muxing a stream of png images into mpeg and FFmpeg > > does that without complaining. > > ./ffmpeg -i matrixbench_mpeg2.mpg -vcodec png test.mpeg > > > > So if we do not check this for a major format that has a proper specification > > which i belive nowhere allows png > > > > Why should we add a limitation on a format that has nothing saying that you > > cannot put some other codec into it ? > > The proper thing to do would be to effectively disallow such invalid > muxing scenarios, fixing this instead of adding even more wrong cases to > the pile. > I'm fairly sure we blocked a patch to allow muxing hevc into flv years > ago. That's what needs to be done in general. I think where we disagree is not that "we should disallow invalid muxing" but if this is invalid/wrong. And also it seems to me that people appear to be very quick in jumping onto such "lets block it" movments without doing much research about new codecs in IVF, which this thread is about, why is that invalid ? our ivf demuxer refers to itself as "On2 IVF" so i presume the format was created by On2 (googling for On2 IVF results in only strange results interrestingly) but presumably IVF was created by On2 In February 2010, On2 Technologies was acquired by Google (https://en.wikipedia.org/wiki/On2_Technologies) And google is using IVF apparently from what was said in this thread with more codecs. Well from how i understand it (please correct me if iam wrong) IVF is now a format "owned" by google. They certainly can add things to it. Theres nothing wrong or invalid on this. Thanks [...]
On Thu, Oct 11, 2018 at 5:28 PM Jan Ekström <jeebjp@gmail.com> wrote: > On Thu, Oct 11, 2018 at 10:58 PM Alex Sukhanov <alx.sukhanov@gmail.com> > wrote: > > > > Hi Mark, > > > > at Google we have some old service which is still running and it works > only > > with the IVF container. It would be great if ffmpeg could generate such > > videos so we could give them to the service then. Given that ffmpeg IVF > > demuxer already supports reading such files, I think it's reasonable to > > make IVF muxer be able to generate them. > > Hope it answers the question. > > > > Thank you > > Given the amount of code is not large I'm not against having it in, > but if it's not something that ever was meant to go into the public > I'd probably disable creation of such files unless you enable a less > standards-compliant strictness mode. > maybe creation of such files could be allowed only with a strict compliance flag?
On Mon, Oct 15, 2018 at 9:35 AM Vittorio Giovara <vittorio.giovara@gmail.com> wrote: > On Thu, Oct 11, 2018 at 5:28 PM Jan Ekström <jeebjp@gmail.com> wrote: > > > On Thu, Oct 11, 2018 at 10:58 PM Alex Sukhanov <alx.sukhanov@gmail.com> > > wrote: > > > > > > Hi Mark, > > > > > > at Google we have some old service which is still running and it works > > only > > > with the IVF container. It would be great if ffmpeg could generate such > > > videos so we could give them to the service then. Given that ffmpeg IVF > > > demuxer already supports reading such files, I think it's reasonable to > > > make IVF muxer be able to generate them. > > > Hope it answers the question. > > > > > > Thank you > > > > Given the amount of code is not large I'm not against having it in, > > but if it's not something that ever was meant to go into the public > > I'd probably disable creation of such files unless you enable a less > > standards-compliant strictness mode. > > > > maybe creation of such files could be allowed only with a strict compliance > flag? > -- > Vittorio > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Vittorio, can you please elaborate your proposal? Is there already a flag in ffmpeg that we can consider use for that? What would be a benefit of guarding h264/hevc muxing into the IVF container by the flag? Thank you
On Sat, Oct 13, 2018 at 5:17 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, Oct 12, 2018 at 01:13:45AM +0100, Derek Buitenhuis wrote: > > On 11/10/2018 23:39, Alex Sukhanov wrote: > > > The only "spec" I'm aware of:https://wiki.multimedia.cx/index.php/IVF > > > > Yeah, not really a spec. > > > > I have no strong objections, I guess. > > so IIUC noone is against this ? > if so i will apply the last patch in a few days unless i forget or > someone else does before > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Everything should be made as simple as possible, but not simpler. > -- Albert Einstein > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Michael, can you please apply this patch?
On Wed, Oct 24, 2018 at 04:50:07PM -0700, Alex Sukhanov wrote: > On Sat, Oct 13, 2018 at 5:17 PM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > On Fri, Oct 12, 2018 at 01:13:45AM +0100, Derek Buitenhuis wrote: > > > On 11/10/2018 23:39, Alex Sukhanov wrote: > > > > The only "spec" I'm aware of:https://wiki.multimedia.cx/index.php/IVF > > > > > > Yeah, not really a spec. > > > > > > I have no strong objections, I guess. > > > > so IIUC noone is against this ? > > if so i will apply the last patch in a few days unless i forget or > > someone else does before > > > > thx > > > > [...] > > -- > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > > > Everything should be made as simple as possible, but not simpler. > > -- Albert Einstein > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > Michael, can you please apply this patch? It seemed to me that jan, james and vittorio where not happy about it It would be better if everyone agreed with what is applied, maybe you can talk with them to see if you can find something that everyone is happy with. Thanks [...]
diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c index 66441a2a43..6410828533 100644 --- a/libavformat/ivfenc.c +++ b/libavformat/ivfenc.c @@ -36,19 +36,29 @@ static int ivf_write_header(AVFormatContext *s) return AVERROR(EINVAL); } par = s->streams[0]->codecpar; - if (par->codec_type != AVMEDIA_TYPE_VIDEO || - !(par->codec_id == AV_CODEC_ID_AV1 || - par->codec_id == AV_CODEC_ID_VP8 || - par->codec_id == AV_CODEC_ID_VP9)) { - av_log(s, AV_LOG_ERROR, "Currently only VP8, VP9 and AV1 are supported!\n"); - return AVERROR(EINVAL); - } avio_write(pb, "DKIF", 4); avio_wl16(pb, 0); // version avio_wl16(pb, 32); // header length - avio_wl32(pb, - par->codec_id == AV_CODEC_ID_VP9 ? AV_RL32("VP90") : - par->codec_id == AV_CODEC_ID_VP8 ? AV_RL32("VP80") : AV_RL32("AV01")); + switch (par->codec_id) { + case AV_CODEC_ID_AV1: + avio_wl32(pb, AV_RL32("AV01")); + break; + case AV_CODEC_ID_H264: + avio_wl32(pb, AV_RL32("H264")); + break; + case AV_CODEC_ID_HEVC: + avio_wl32(pb, AV_RL32("HEVC")); + break; + case AV_CODEC_ID_VP8: + avio_wl32(pb, AV_RL32("VP80")); + break; + case AV_CODEC_ID_VP9: + avio_wl32(pb, AV_RL32("VP90")); + break; + default: + av_log(s, AV_LOG_ERROR, "Currently only AV1, H264, HEVC, VP8 and VP9 and AV1 are supported!\n"); + return AVERROR(EINVAL); + } avio_wl16(pb, par->width); avio_wl16(pb, par->height); avio_wl32(pb, s->streams[0]->time_base.den); @@ -95,16 +105,32 @@ static int ivf_check_bitstream(struct AVFormatContext *s, const AVPacket *pkt) int ret = 1; AVStream *st = s->streams[pkt->stream_index]; - if (st->codecpar->codec_id == AV_CODEC_ID_VP9) + if (st->codecpar->codec_id == AV_CODEC_ID_H264) { + if (pkt->size >= 5 && AV_RB32(pkt->data) != 0x0000001 && + (AV_RB24(pkt->data) != 0x000001 || + (st->codecpar->extradata_size > 0 && + st->codecpar->extradata[0] == 1))) + ret = ff_stream_add_bitstream_filter(st, "h264_mp4toannexb", NULL); + } else if (st->codecpar->codec_id == AV_CODEC_ID_HEVC) { + if (pkt->size >= 5 && AV_RB32(pkt->data) != 0x0000001 && + (AV_RB24(pkt->data) != 0x000001 || + (st->codecpar->extradata_size > 0 && + st->codecpar->extradata[0] == 1))) + ret = ff_stream_add_bitstream_filter(st, "hevc_mp4toannexb", NULL); + } else if (st->codecpar->codec_id == AV_CODEC_ID_VP9) { ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", NULL); + } return ret; } static const AVCodecTag codec_ivf_tags[] = { + { AV_CODEC_ID_AV1, MKTAG('A', 'V', '0', '1') }, + { AV_CODEC_ID_H264, MKTAG('H', '2', '6', '4') }, + { AV_CODEC_ID_HEVC, MKTAG('H', 'E', 'V', 'C') }, { AV_CODEC_ID_VP8, MKTAG('V', 'P', '8', '0') }, { AV_CODEC_ID_VP9, MKTAG('V', 'P', '9', '0') }, - { AV_CODEC_ID_AV1, MKTAG('A', 'V', '0', '1') }, + { AV_CODEC_ID_NONE, 0 } };
From: Alex Sukhanov <asukhanov@google.com> --- libavformat/ivfenc.c | 50 +++++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 12 deletions(-)