| Message ID | 20181003001805.9312-2-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On 03/10/18 01:18, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/ivfenc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c > index 66441a2a43..adf72117e9 100644 > --- a/libavformat/ivfenc.c > +++ b/libavformat/ivfenc.c > @@ -97,6 +97,8 @@ static int ivf_check_bitstream(struct AVFormatContext *s, const AVPacket *pkt) > > if (st->codecpar->codec_id == AV_CODEC_ID_VP9) > ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", NULL); > + else if (st->codecpar->codec_id == AV_CODEC_ID_AV1) > + ret = ff_stream_add_bitstream_filter(st, "av1_metadata", "td=insert"); > > return ret; > } > I'm not quite seeing why this is wanted - could you explain it a bit further? (Since IVF is packetised into temporal units already, it seems to me that having TDs or not in the file won't change anything from the point of view of the consumer.) Thanks, - Mark
On 10/3/2018 8:01 PM, Mark Thompson wrote: > On 03/10/18 01:18, James Almer wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/ivfenc.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c >> index 66441a2a43..adf72117e9 100644 >> --- a/libavformat/ivfenc.c >> +++ b/libavformat/ivfenc.c >> @@ -97,6 +97,8 @@ static int ivf_check_bitstream(struct AVFormatContext *s, const AVPacket *pkt) >> >> if (st->codecpar->codec_id == AV_CODEC_ID_VP9) >> ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", NULL); >> + else if (st->codecpar->codec_id == AV_CODEC_ID_AV1) >> + ret = ff_stream_add_bitstream_filter(st, "av1_metadata", "td=insert"); >> >> return ret; >> } >> > > I'm not quite seeing why this is wanted - could you explain it a bit further? > > (Since IVF is packetised into temporal units already, it seems to me that having TDs or not in the file won't change anything from the point of view of the consumer.) No, but ivf afaik doesn't suggest or mandate the removal of TDs, unlike mp4 and matroska, so i figured it would be best to put them back in place for such remuxing scenarios. I don't mind dropping this patch. > > Thanks, > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 04/10/18 00:12, James Almer wrote: > On 10/3/2018 8:01 PM, Mark Thompson wrote: >> On 03/10/18 01:18, James Almer wrote: >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavformat/ivfenc.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c >>> index 66441a2a43..adf72117e9 100644 >>> --- a/libavformat/ivfenc.c >>> +++ b/libavformat/ivfenc.c >>> @@ -97,6 +97,8 @@ static int ivf_check_bitstream(struct AVFormatContext *s, const AVPacket *pkt) >>> >>> if (st->codecpar->codec_id == AV_CODEC_ID_VP9) >>> ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", NULL); >>> + else if (st->codecpar->codec_id == AV_CODEC_ID_AV1) >>> + ret = ff_stream_add_bitstream_filter(st, "av1_metadata", "td=insert"); >>> >>> return ret; >>> } >>> >> >> I'm not quite seeing why this is wanted - could you explain it a bit further? >> >> (Since IVF is packetised into temporal units already, it seems to me that having TDs or not in the file won't change anything from the point of view of the consumer.) > > No, but ivf afaik doesn't suggest or mandate the removal of TDs, unlike > mp4 and matroska, so i figured it would be best to put them back in > place for such remuxing scenarios. > > I don't mind dropping this patch. No preference - I don't have any arguments one way or the other. If you think it's a good idea then it looks fine to me, though probably the IVF muxer needs to select av1_metadata in configure to match. (Several muxers seem to be missing these dependencies, including IVF on vp9_superframe - patch incoming.) Thanks, - Mark
On 10/4/2018 7:48 PM, Mark Thompson wrote: > On 04/10/18 00:12, James Almer wrote: >> On 10/3/2018 8:01 PM, Mark Thompson wrote: >>> On 03/10/18 01:18, James Almer wrote: >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> libavformat/ivfenc.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c >>>> index 66441a2a43..adf72117e9 100644 >>>> --- a/libavformat/ivfenc.c >>>> +++ b/libavformat/ivfenc.c >>>> @@ -97,6 +97,8 @@ static int ivf_check_bitstream(struct AVFormatContext *s, const AVPacket *pkt) >>>> >>>> if (st->codecpar->codec_id == AV_CODEC_ID_VP9) >>>> ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", NULL); >>>> + else if (st->codecpar->codec_id == AV_CODEC_ID_AV1) >>>> + ret = ff_stream_add_bitstream_filter(st, "av1_metadata", "td=insert"); >>>> >>>> return ret; >>>> } >>>> >>> >>> I'm not quite seeing why this is wanted - could you explain it a bit further? >>> >>> (Since IVF is packetised into temporal units already, it seems to me that having TDs or not in the file won't change anything from the point of view of the consumer.) >> >> No, but ivf afaik doesn't suggest or mandate the removal of TDs, unlike >> mp4 and matroska, so i figured it would be best to put them back in >> place for such remuxing scenarios. >> >> I don't mind dropping this patch. > > No preference - I don't have any arguments one way or the other. > > If you think it's a good idea then it looks fine to me, though probably the IVF muxer needs to select av1_metadata in configure to match. (Several muxers seem to be missing these dependencies, including IVF on vp9_superframe - patch incoming.) > > Thanks, > > - Mark aomenc doesn't remove them, and other muxers like mkvextract readd them if missing as well, so configure dep added, and pushed. Thanks!
diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c index 66441a2a43..adf72117e9 100644 --- a/libavformat/ivfenc.c +++ b/libavformat/ivfenc.c @@ -97,6 +97,8 @@ static int ivf_check_bitstream(struct AVFormatContext *s, const AVPacket *pkt) if (st->codecpar->codec_id == AV_CODEC_ID_VP9) ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", NULL); + else if (st->codecpar->codec_id == AV_CODEC_ID_AV1) + ret = ff_stream_add_bitstream_filter(st, "av1_metadata", "td=insert"); return ret; }
Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/ivfenc.c | 2 ++ 1 file changed, 2 insertions(+)