| Message ID | 20191114171534.258181-1-tfoucu@gmail.com |
|---|---|
| State | Withdrawn |
| Headers | show |
On 11/14/2019 2:15 PM, Thierry Foucu wrote: > Set the option to false by default, to match libaomdec wrapper. > --- > libavcodec/libdav1d.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c > index cf4b178f1d..5efa8eeb48 100644 > --- a/libavcodec/libdav1d.c > +++ b/libavcodec/libdav1d.c > @@ -40,6 +40,7 @@ typedef struct Libdav1dContext { > int tile_threads; > int frame_threads; > int apply_grain; > + int all_layers; > } Libdav1dContext; > > static const enum AVPixelFormat pix_fmt[][3] = { > @@ -134,6 +135,8 @@ static av_cold int libdav1d_init(AVCodecContext *c) > if (dav1d->apply_grain >= 0) > s.apply_grain = dav1d->apply_grain; > > + s.all_layers = dav1d->all_layers; > + > s.n_tile_threads = dav1d->tile_threads > ? dav1d->tile_threads > : FFMIN(floor(sqrt(threads)), DAV1D_MAX_TILE_THREADS); > @@ -378,6 +381,7 @@ static const AVOption libdav1d_options[] = { > { "tilethreads", "Tile threads", OFFSET(tile_threads), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_TILE_THREADS, VD }, > { "framethreads", "Frame threads", OFFSET(frame_threads), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_FRAME_THREADS, VD }, > { "filmgrain", "Apply Film Grain", OFFSET(apply_grain), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VD }, > + { "alllayers", "Output all spatial layers", OFFSET(all_layers), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, > { NULL } > }; IMO, we don't want to output all layers under any circumstance. It'll result in a mix of frames with duplicate pts and different dimensions. What you could add is a new option "oppoint" that maps to Dav1dSettings operating_point. Set it to -1, and handle it the same way as filmgrain in libdav1d_init(), and then unconditionally set all_layers to 0.
On Thu, Nov 14, 2019 at 9:20 AM James Almer <jamrial@gmail.com> wrote: > On 11/14/2019 2:15 PM, Thierry Foucu wrote: > > Set the option to false by default, to match libaomdec wrapper. > > --- > > libavcodec/libdav1d.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c > > index cf4b178f1d..5efa8eeb48 100644 > > --- a/libavcodec/libdav1d.c > > +++ b/libavcodec/libdav1d.c > > @@ -40,6 +40,7 @@ typedef struct Libdav1dContext { > > int tile_threads; > > int frame_threads; > > int apply_grain; > > + int all_layers; > > } Libdav1dContext; > > > > static const enum AVPixelFormat pix_fmt[][3] = { > > @@ -134,6 +135,8 @@ static av_cold int libdav1d_init(AVCodecContext *c) > > if (dav1d->apply_grain >= 0) > > s.apply_grain = dav1d->apply_grain; > > > > + s.all_layers = dav1d->all_layers; > > + > > s.n_tile_threads = dav1d->tile_threads > > ? dav1d->tile_threads > > : FFMIN(floor(sqrt(threads)), > DAV1D_MAX_TILE_THREADS); > > @@ -378,6 +381,7 @@ static const AVOption libdav1d_options[] = { > > { "tilethreads", "Tile threads", OFFSET(tile_threads), > AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_TILE_THREADS, VD }, > > { "framethreads", "Frame threads", OFFSET(frame_threads), > AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_FRAME_THREADS, VD }, > > { "filmgrain", "Apply Film Grain", OFFSET(apply_grain), > AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VD }, > > + { "alllayers", "Output all spatial layers", OFFSET(all_layers), > AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, > > { NULL } > > }; > > IMO, we don't want to output all layers under any circumstance. It'll > result in a mix of frames with duplicate pts and different dimensions. > > What you could add is a new option "oppoint" that maps to Dav1dSettings > operating_point. Set it to -1, and handle it the same way as filmgrain > in libdav1d_init(), and then unconditionally set all_layers to 0. > I think to have an option to output all the layers is still a good idea if you need to debug some layers issues. especially if you need to run some conformance testing and debug each layer. But i will add the other option to select the layer. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 2019-11-14 20:19, James Almer wrote: > On 11/14/2019 2:15 PM, Thierry Foucu wrote: >> Set the option to false by default, to match libaomdec wrapper. >> --- >> libavcodec/libdav1d.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c >> index cf4b178f1d..5efa8eeb48 100644 >> --- a/libavcodec/libdav1d.c >> +++ b/libavcodec/libdav1d.c >> @@ -40,6 +40,7 @@ typedef struct Libdav1dContext { >> int tile_threads; >> int frame_threads; >> int apply_grain; >> + int all_layers; >> } Libdav1dContext; >> >> static const enum AVPixelFormat pix_fmt[][3] = { >> @@ -134,6 +135,8 @@ static av_cold int libdav1d_init(AVCodecContext *c) >> if (dav1d->apply_grain >= 0) >> s.apply_grain = dav1d->apply_grain; >> >> + s.all_layers = dav1d->all_layers; >> + >> s.n_tile_threads = dav1d->tile_threads >> ? dav1d->tile_threads >> : FFMIN(floor(sqrt(threads)), DAV1D_MAX_TILE_THREADS); >> @@ -378,6 +381,7 @@ static const AVOption libdav1d_options[] = { >> { "tilethreads", "Tile threads", OFFSET(tile_threads), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_TILE_THREADS, VD }, >> { "framethreads", "Frame threads", OFFSET(frame_threads), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_FRAME_THREADS, VD }, >> { "filmgrain", "Apply Film Grain", OFFSET(apply_grain), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VD }, >> + { "alllayers", "Output all spatial layers", OFFSET(all_layers), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, >> { NULL } >> }; > > IMO, we don't want to output all layers under any circumstance. It'll > result in a mix of frames with duplicate pts and different dimensions. IIRC, for VP9 with spatial SVC lavc decoder worked exactly like that - by returning a frame per spatial layer. Is this considered a bug?
On 11/14/2019 2:31 PM, Andrey Semashev wrote: > On 2019-11-14 20:19, James Almer wrote: >> On 11/14/2019 2:15 PM, Thierry Foucu wrote: >>> Set the option to false by default, to match libaomdec wrapper. >>> --- >>> libavcodec/libdav1d.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c >>> index cf4b178f1d..5efa8eeb48 100644 >>> --- a/libavcodec/libdav1d.c >>> +++ b/libavcodec/libdav1d.c >>> @@ -40,6 +40,7 @@ typedef struct Libdav1dContext { >>> int tile_threads; >>> int frame_threads; >>> int apply_grain; >>> + int all_layers; >>> } Libdav1dContext; >>> static const enum AVPixelFormat pix_fmt[][3] = { >>> @@ -134,6 +135,8 @@ static av_cold int libdav1d_init(AVCodecContext *c) >>> if (dav1d->apply_grain >= 0) >>> s.apply_grain = dav1d->apply_grain; >>> + s.all_layers = dav1d->all_layers; >>> + >>> s.n_tile_threads = dav1d->tile_threads >>> ? dav1d->tile_threads >>> : FFMIN(floor(sqrt(threads)), >>> DAV1D_MAX_TILE_THREADS); >>> @@ -378,6 +381,7 @@ static const AVOption libdav1d_options[] = { >>> { "tilethreads", "Tile threads", OFFSET(tile_threads), >>> AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_TILE_THREADS, VD }, >>> { "framethreads", "Frame threads", OFFSET(frame_threads), >>> AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_FRAME_THREADS, VD }, >>> { "filmgrain", "Apply Film Grain", OFFSET(apply_grain), >>> AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VD }, >>> + { "alllayers", "Output all spatial layers", OFFSET(all_layers), >>> AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, >>> { NULL } >>> }; >> >> IMO, we don't want to output all layers under any circumstance. It'll >> result in a mix of frames with duplicate pts and different dimensions. > > IIRC, for VP9 with spatial SVC lavc decoder worked exactly like that - > by returning a frame per spatial layer. Is this considered a bug? I don't know if it's a bug, and never seen or tested vp9 svc samples before. Ronald might know. But it sounds like having more than one video "stream" per AVCodecContext, which i don't think was intended. And if it was, then it would be up to the library user to figure what to do with these frames.
On 2019-11-14 20:48, James Almer wrote: > On 11/14/2019 2:31 PM, Andrey Semashev wrote: >> On 2019-11-14 20:19, James Almer wrote: >>> On 11/14/2019 2:15 PM, Thierry Foucu wrote: >>>> Set the option to false by default, to match libaomdec wrapper. >>>> --- >>>> libavcodec/libdav1d.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c >>>> index cf4b178f1d..5efa8eeb48 100644 >>>> --- a/libavcodec/libdav1d.c >>>> +++ b/libavcodec/libdav1d.c >>>> @@ -40,6 +40,7 @@ typedef struct Libdav1dContext { >>>> int tile_threads; >>>> int frame_threads; >>>> int apply_grain; >>>> + int all_layers; >>>> } Libdav1dContext; >>>> static const enum AVPixelFormat pix_fmt[][3] = { >>>> @@ -134,6 +135,8 @@ static av_cold int libdav1d_init(AVCodecContext *c) >>>> if (dav1d->apply_grain >= 0) >>>> s.apply_grain = dav1d->apply_grain; >>>> + s.all_layers = dav1d->all_layers; >>>> + >>>> s.n_tile_threads = dav1d->tile_threads >>>> ? dav1d->tile_threads >>>> : FFMIN(floor(sqrt(threads)), >>>> DAV1D_MAX_TILE_THREADS); >>>> @@ -378,6 +381,7 @@ static const AVOption libdav1d_options[] = { >>>> { "tilethreads", "Tile threads", OFFSET(tile_threads), >>>> AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_TILE_THREADS, VD }, >>>> { "framethreads", "Frame threads", OFFSET(frame_threads), >>>> AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_FRAME_THREADS, VD }, >>>> { "filmgrain", "Apply Film Grain", OFFSET(apply_grain), >>>> AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VD }, >>>> + { "alllayers", "Output all spatial layers", OFFSET(all_layers), >>>> AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, >>>> { NULL } >>>> }; >>> >>> IMO, we don't want to output all layers under any circumstance. It'll >>> result in a mix of frames with duplicate pts and different dimensions. >> >> IIRC, for VP9 with spatial SVC lavc decoder worked exactly like that - >> by returning a frame per spatial layer. Is this considered a bug? > > I don't know if it's a bug, and never seen or tested vp9 svc samples > before. Ronald might know. But it sounds like having more than one video > "stream" per AVCodecContext, which i don't think was intended. There is one stream, in which an encoded frame is a VP9 superframe that contains frames for each spatial layer. From the user's perspective, VP9 superframe is one entity. > And if it was, then it would be up to the library user to figure what to > do with these frames. I think there needs to be some consistency across different lavc decoders. If we consider that lavc should produce one decoded frame per one encoded one, even if the encoded one contains multiple layers, then that should be true for all decoders. Also, I think having decoded frames from all layers would also be useful, but there should be a way to know which layer they belong to. AFAIK, lavc currently doesn't provide that information. This mode of operation (producing frames for all layers) should be optional.
Hi, On Thu, Nov 14, 2019 at 2:12 PM Andrey Semashev <andrey.semashev@gmail.com> wrote: > I think there needs to be some consistency across different lavc > decoders. If we consider that lavc should produce one decoded frame per > one encoded one, even if the encoded one contains multiple layers, then > that should be true for all decoders. > Yes. I think one thing that would help is if we had access to more samples with an expected behaviour. Right now we may have samples, but if all we do is check md5 without caring what it means, then it's kind of pointless. > Also, I think having decoded frames from all layers would also be > useful, but there should be a way to know which layer they belong to. > AFAIK, lavc currently doesn't provide that information. This mode of > operation (producing frames for all layers) should be optional. I agree. Ronald
On Thu, Nov 14, 2019 at 11:33 AM Ronald S. Bultje <rsbultje@gmail.com> wrote: > Hi, > > On Thu, Nov 14, 2019 at 2:12 PM Andrey Semashev <andrey.semashev@gmail.com > > > wrote: > > > I think there needs to be some consistency across different lavc > > decoders. If we consider that lavc should produce one decoded frame per > > one encoded one, even if the encoded one contains multiple layers, then > > that should be true for all decoders. > > > > Yes. I think one thing that would help is if we had access to more samples > with an expected behaviour. Right now we may have samples, but if all we do > is check md5 without caring what it means, then it's kind of pointless. > > > > Also, I think having decoded frames from all layers would also be > > useful, but there should be a way to know which layer they belong to. > > AFAIK, lavc currently doesn't provide that information. This mode of > > operation (producing frames for all layers) should be optional. > > > I agree. > I think this is what my second patch is doing. Set by default to false to output all the layer and add an option to select which operating point we want the decoder to output. > > Ronald > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c index cf4b178f1d..5efa8eeb48 100644 --- a/libavcodec/libdav1d.c +++ b/libavcodec/libdav1d.c @@ -40,6 +40,7 @@ typedef struct Libdav1dContext { int tile_threads; int frame_threads; int apply_grain; + int all_layers; } Libdav1dContext; static const enum AVPixelFormat pix_fmt[][3] = { @@ -134,6 +135,8 @@ static av_cold int libdav1d_init(AVCodecContext *c) if (dav1d->apply_grain >= 0) s.apply_grain = dav1d->apply_grain; + s.all_layers = dav1d->all_layers; + s.n_tile_threads = dav1d->tile_threads ? dav1d->tile_threads : FFMIN(floor(sqrt(threads)), DAV1D_MAX_TILE_THREADS); @@ -378,6 +381,7 @@ static const AVOption libdav1d_options[] = { { "tilethreads", "Tile threads", OFFSET(tile_threads), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_TILE_THREADS, VD }, { "framethreads", "Frame threads", OFFSET(frame_threads), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_FRAME_THREADS, VD }, { "filmgrain", "Apply Film Grain", OFFSET(apply_grain), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VD }, + { "alllayers", "Output all spatial layers", OFFSET(all_layers), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, { NULL } };