| Message ID | 90b8080b-bccd-81cb-04d4-935f2b1c8a8f@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 4/6/2018 8:34 PM, Gyan Doshi wrote: > > On 4/5/2018 12:09 AM, Michael Niedermayer wrote: >> >> This does not work >> breaks fate-unknown_layout-ac3 >> >> also the tests mayb too strict, there are similar codec_ids like mpeg1/2 >> or jpeg variants which are all basically the same from a muxers point >> of view > > Revised patch passes FATE. Set looser conditions for mpeg-1/2/4 and jpeg > muxer. Ping.
On Fri, Apr 06, 2018 at 08:34:12PM +0530, Gyan Doshi wrote: > > On 4/5/2018 12:09 AM, Michael Niedermayer wrote: > > > >This does not work > >breaks fate-unknown_layout-ac3 > > > >also the tests mayb too strict, there are similar codec_ids like mpeg1/2 > >or jpeg variants which are all basically the same from a muxers point of view > > Revised patch passes FATE. Set looser conditions for mpeg-1/2/4 and jpeg > muxer. > > Regards, > Gyan > rawenc.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > 5aaae3c1bf40a983cff15bc2fc603ac214d9437a v2-0001-avformat-rawenc-check-stream-type.patch > From a4ae3eb20af9a4c6e5feb201347b5b044541c59b Mon Sep 17 00:00:00 2001 > From: Gyan Doshi <gyandoshi@gmail.com> > Date: Fri, 6 Apr 2018 20:21:57 +0530 > Subject: [PATCH v2] avformat/rawenc: check stream type > > Validate codec of stream to be muxed except for data muxer. > --- > libavformat/rawenc.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/libavformat/rawenc.c b/libavformat/rawenc.c > index 809ca23b1a..19028329f7 100644 > --- a/libavformat/rawenc.c > +++ b/libavformat/rawenc.c > @@ -34,12 +34,48 @@ int ff_raw_write_packet(AVFormatContext *s, AVPacket *pkt) > > static int force_one_stream(AVFormatContext *s) > { > +enum AVCodecID id; > +const char *idname; indention is wrong > + > if (s->nb_streams != 1) { > av_log(s, AV_LOG_ERROR, "%s files have exactly one stream\n", > s->oformat->name); > return AVERROR(EINVAL); > } > + > + if (!strcmp("data", s->oformat->name)) > + return 0; > + > +id = s->streams[0]->codecpar->codec_id; > +idname = avcodec_get_name(id); same > + > + switch(s->streams[0]->codecpar->codec_type) { > + case AVMEDIA_TYPE_AUDIO: > + if (s->oformat->audio_codec != id) > + goto fail; > + break; > + case AVMEDIA_TYPE_VIDEO: > + if (strstr(s->oformat->name, "mpeg") != NULL) { > + if (strstr(idname, "mpeg") == NULL) > + goto fail; > + } else if (strstr(s->oformat->name, "m4v") != NULL) { > + if (strstr(idname, "mpeg4") == NULL) > + goto fail; > + } else if (strstr(s->oformat->name, "jpeg") != NULL) { > + if (strstr(idname, "jpeg") == NULL) > + goto fail; > + } else if (s->oformat->video_codec != id) > + goto fail; > + break; iam not sure these cross similarity stuff should be hardcoded here. It seems painfull to have to maintain this. also the ones i mentioned might not be all cases To implement generic "codec supported" checks, muxers would have to list what they support. There are 3 ways i know how this can be done in the current API 1. the audio_codec / video_codec / ... fields 2. the codec_tag array (this was in fact IIRC one of the intended uses of it) 3. the querry_format callback thanks [...]
On 4/10/2018 4:49 AM, Michael Niedermayer wrote: > To implement generic "codec supported" checks, muxers would have to > list what they support. > There are 3 ways i know how this can be done in the current API > 1. the audio_codec / video_codec / ... fields > 2. the codec_tag array (this was in fact IIRC one of the intended uses of it) > 3. the querry_format callback These are raw single stream muxers - most only dump packets and _should_ accept exactly one codec_id. A couple may insert bitstream filters. Only one writes a trailer. So none of them have a codec_tag member or a query_codec callback. Personally, I agree that the current method is inelegant but these "muxers" are stable and these tests shouldn't need changes. But I can go the query_codec route if you advise it. Regards, Gyan
On Tue, Apr 10, 2018 at 07:21:29PM +0530, Gyan Doshi wrote: > > > On 4/10/2018 4:49 AM, Michael Niedermayer wrote: > > >To implement generic "codec supported" checks, muxers would have to > >list what they support. > >There are 3 ways i know how this can be done in the current API > >1. the audio_codec / video_codec / ... fields > >2. the codec_tag array (this was in fact IIRC one of the intended uses of it) > >3. the querry_format callback > > These are raw single stream muxers - most only dump packets and _should_ > accept exactly one codec_id. A couple may insert bitstream filters. Only one > writes a trailer. So none of them have a codec_tag member or a query_codec > callback. Personally, I agree that the current method is inelegant but these > "muxers" are stable and these tests shouldn't need changes. But I can go the > query_codec route if you advise it. yes please do note query_codec is only needed for the few special cases that do more than one codec_id thx [...]
diff --git a/libavformat/rawenc.c b/libavformat/rawenc.c index 809ca23b1a..19028329f7 100644 --- a/libavformat/rawenc.c +++ b/libavformat/rawenc.c @@ -34,12 +34,48 @@ int ff_raw_write_packet(AVFormatContext *s, AVPacket *pkt) static int force_one_stream(AVFormatContext *s) { +enum AVCodecID id; +const char *idname; + if (s->nb_streams != 1) { av_log(s, AV_LOG_ERROR, "%s files have exactly one stream\n", s->oformat->name); return AVERROR(EINVAL); } + + if (!strcmp("data", s->oformat->name)) + return 0; + +id = s->streams[0]->codecpar->codec_id; +idname = avcodec_get_name(id); + + switch(s->streams[0]->codecpar->codec_type) { + case AVMEDIA_TYPE_AUDIO: + if (s->oformat->audio_codec != id) + goto fail; + break; + case AVMEDIA_TYPE_VIDEO: + if (strstr(s->oformat->name, "mpeg") != NULL) { + if (strstr(idname, "mpeg") == NULL) + goto fail; + } else if (strstr(s->oformat->name, "m4v") != NULL) { + if (strstr(idname, "mpeg4") == NULL) + goto fail; + } else if (strstr(s->oformat->name, "jpeg") != NULL) { + if (strstr(idname, "jpeg") == NULL) + goto fail; + } else if (s->oformat->video_codec != id) + goto fail; + break; + default: + goto fail; + } + return 0; + +fail: + av_log(s, AV_LOG_ERROR, "Stream not of type %s\n", s->oformat->name); + return AVERROR(EINVAL); } /* Note: Do not forget to add new entries to the Makefile as well. */