| Message ID | 20200706183007.2472127-1-yonglel@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] libavcodec/vp9: export block structure when segmentation isn't enable | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Mon, Jul 6, 2020 at 11:31 AM Yongle Lin <yongle.lin.94@gmail.com> wrote: > it makes sense to export block structure like src_x, src_y, width and > height when segmentation isn't enable so we could visualize and see the > structure of the block. > --- > libavcodec/vp9.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c > index fd0bab14a2..e700def70e 100644 > --- a/libavcodec/vp9.c > +++ b/libavcodec/vp9.c > @@ -1501,10 +1501,8 @@ static int vp9_export_enc_params(VP9Context *s, > VP9Frame *frame) > AVVideoEncParams *par; > unsigned int tile, nb_blocks = 0; > > - if (s->s.h.segmentation.enabled) { > - for (tile = 0; tile < s->active_tile_cols; tile++) > - nb_blocks += s->td[tile].nb_block_structure; > - } > + for (tile = 0; tile < s->active_tile_cols; tile++) > + nb_blocks += s->td[tile].nb_block_structure; > > par = av_video_enc_params_create_side_data(frame->tf.f, > AV_VIDEO_ENC_PARAMS_VP9, nb_blocks); > @@ -1536,7 +1534,7 @@ static int vp9_export_enc_params(VP9Context *s, > VP9Frame *frame) > b->w = 1 << (3 + > td->block_structure[block_tile].block_size_idx_x); > b->h = 1 << (3 + > td->block_structure[block_tile].block_size_idx_y); > > - if (s->s.h.segmentation.feat[seg_id].q_enabled) { > + if (s->s.h.segmentation.enabled && > s->s.h.segmentation.feat[seg_id].q_enabled) { > b->delta_qp = s->s.h.segmentation.feat[seg_id].q_val; > if (s->s.h.segmentation.absolute_vals) > b->delta_qp -= par->qp; > -- > 2.27.0.383.g050319c2ae-goog > > Dear FFmpeg Developers, Currently ffmpeg doesn't export the block data for VP9 if there is no segmentation. Because it's only used to export QP value. I think it makes more sense to export the block information without segmentation so we could visualize the block structure for VP9 video. Could you please review and merge this patch? Thanks. Best, Yongle
On Thu, Jul 9, 2020 at 2:23 PM Yongle Lin <yonglel@google.com> wrote: > > > On Mon, Jul 6, 2020 at 11:31 AM Yongle Lin <yongle.lin.94@gmail.com> > wrote: > >> it makes sense to export block structure like src_x, src_y, width and >> height when segmentation isn't enable so we could visualize and see the >> structure of the block. >> --- >> libavcodec/vp9.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c >> index fd0bab14a2..e700def70e 100644 >> --- a/libavcodec/vp9.c >> +++ b/libavcodec/vp9.c >> @@ -1501,10 +1501,8 @@ static int vp9_export_enc_params(VP9Context *s, >> VP9Frame *frame) >> AVVideoEncParams *par; >> unsigned int tile, nb_blocks = 0; >> >> - if (s->s.h.segmentation.enabled) { >> - for (tile = 0; tile < s->active_tile_cols; tile++) >> - nb_blocks += s->td[tile].nb_block_structure; >> - } >> + for (tile = 0; tile < s->active_tile_cols; tile++) >> + nb_blocks += s->td[tile].nb_block_structure; >> >> par = av_video_enc_params_create_side_data(frame->tf.f, >> AV_VIDEO_ENC_PARAMS_VP9, nb_blocks); >> @@ -1536,7 +1534,7 @@ static int vp9_export_enc_params(VP9Context *s, >> VP9Frame *frame) >> b->w = 1 << (3 + >> td->block_structure[block_tile].block_size_idx_x); >> b->h = 1 << (3 + >> td->block_structure[block_tile].block_size_idx_y); >> >> - if (s->s.h.segmentation.feat[seg_id].q_enabled) { >> + if (s->s.h.segmentation.enabled && >> s->s.h.segmentation.feat[seg_id].q_enabled) { >> b->delta_qp = s->s.h.segmentation.feat[seg_id].q_val; >> if (s->s.h.segmentation.absolute_vals) >> b->delta_qp -= par->qp; >> -- >> 2.27.0.383.g050319c2ae-goog >> >> > Dear FFmpeg Developers, > > Currently ffmpeg doesn't export the block data for VP9 if there is no > segmentation. Because it's only used to export QP value. I think it makes > more sense to export the block information without segmentation so we could > visualize the block structure for VP9 video. > > Could you please review and merge this patch? Thanks. > > Best, > Yongle > Dear FFmpeg Developers, Currently ffmpeg doesn't export the block data for VP9 if there is no segmentation. Because it's only used to export QP value. I think it makes more sense to export the block information without segmentation so we could visualize the block structure for VP9 video. Could you please review and merge this patch? Thanks. Best, Yongle
On Mon, Jul 13, 2020 at 2:57 PM Yongle Lin <yongle.lin.94@gmail.com> wrote: > > On Thu, Jul 9, 2020 at 2:23 PM Yongle Lin <yonglel@google.com> wrote: > >> >> >> On Mon, Jul 6, 2020 at 11:31 AM Yongle Lin <yongle.lin.94@gmail.com> >> wrote: >> >>> it makes sense to export block structure like src_x, src_y, width and >>> height when segmentation isn't enable so we could visualize and see the >>> structure of the block. >>> --- >>> libavcodec/vp9.c | 8 +++----- >>> 1 file changed, 3 insertions(+), 5 deletions(-) >>> >>> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c >>> index fd0bab14a2..e700def70e 100644 >>> --- a/libavcodec/vp9.c >>> +++ b/libavcodec/vp9.c >>> @@ -1501,10 +1501,8 @@ static int vp9_export_enc_params(VP9Context *s, >>> VP9Frame *frame) >>> AVVideoEncParams *par; >>> unsigned int tile, nb_blocks = 0; >>> >>> - if (s->s.h.segmentation.enabled) { >>> - for (tile = 0; tile < s->active_tile_cols; tile++) >>> - nb_blocks += s->td[tile].nb_block_structure; >>> - } >>> + for (tile = 0; tile < s->active_tile_cols; tile++) >>> + nb_blocks += s->td[tile].nb_block_structure; >>> >>> par = av_video_enc_params_create_side_data(frame->tf.f, >>> AV_VIDEO_ENC_PARAMS_VP9, nb_blocks); >>> @@ -1536,7 +1534,7 @@ static int vp9_export_enc_params(VP9Context *s, >>> VP9Frame *frame) >>> b->w = 1 << (3 + >>> td->block_structure[block_tile].block_size_idx_x); >>> b->h = 1 << (3 + >>> td->block_structure[block_tile].block_size_idx_y); >>> >>> - if (s->s.h.segmentation.feat[seg_id].q_enabled) { >>> + if (s->s.h.segmentation.enabled && >>> s->s.h.segmentation.feat[seg_id].q_enabled) { >>> b->delta_qp = >>> s->s.h.segmentation.feat[seg_id].q_val; >>> if (s->s.h.segmentation.absolute_vals) >>> b->delta_qp -= par->qp; >>> -- >>> 2.27.0.383.g050319c2ae-goog >>> >>> >> Dear FFmpeg Developers, >> >> Currently ffmpeg doesn't export the block data for VP9 if there is no >> segmentation. Because it's only used to export QP value. I think it makes >> more sense to export the block information without segmentation so we could >> visualize the block structure for VP9 video. >> >> Could you please review and merge this patch? Thanks. >> >> Best, >> Yongle >> > > Dear FFmpeg Developers, > > Currently ffmpeg doesn't export the block data for VP9 if there is no > segmentation. Because it's only used to export QP value. I think it makes > more sense to export the block information without segmentation so we could > visualize the block structure for VP9 video. > > Could you please review and merge this patch? Thanks. > > Best, > Yongle > Hi FFmpeg Developers, I noticed that this patch hasn't been reviewed yet. Could someone help me review it and merge this patch? Thanks. Best, Yongle
On Wed, Aug 26, 2020 at 09:40:19AM -0700, Yongle Lin wrote: > On Mon, Jul 13, 2020 at 2:57 PM Yongle Lin <yongle.lin.94@gmail.com> wrote: > > > > > On Thu, Jul 9, 2020 at 2:23 PM Yongle Lin <yonglel@google.com> wrote: > > > >> > >> > >> On Mon, Jul 6, 2020 at 11:31 AM Yongle Lin <yongle.lin.94@gmail.com> > >> wrote: > >> > >>> it makes sense to export block structure like src_x, src_y, width and > >>> height when segmentation isn't enable so we could visualize and see the > >>> structure of the block. > >>> --- > >>> libavcodec/vp9.c | 8 +++----- > >>> 1 file changed, 3 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c > >>> index fd0bab14a2..e700def70e 100644 > >>> --- a/libavcodec/vp9.c > >>> +++ b/libavcodec/vp9.c > >>> @@ -1501,10 +1501,8 @@ static int vp9_export_enc_params(VP9Context *s, > >>> VP9Frame *frame) > >>> AVVideoEncParams *par; > >>> unsigned int tile, nb_blocks = 0; > >>> > >>> - if (s->s.h.segmentation.enabled) { > >>> - for (tile = 0; tile < s->active_tile_cols; tile++) > >>> - nb_blocks += s->td[tile].nb_block_structure; > >>> - } > >>> + for (tile = 0; tile < s->active_tile_cols; tile++) > >>> + nb_blocks += s->td[tile].nb_block_structure; > >>> > >>> par = av_video_enc_params_create_side_data(frame->tf.f, > >>> AV_VIDEO_ENC_PARAMS_VP9, nb_blocks); > >>> @@ -1536,7 +1534,7 @@ static int vp9_export_enc_params(VP9Context *s, > >>> VP9Frame *frame) > >>> b->w = 1 << (3 + > >>> td->block_structure[block_tile].block_size_idx_x); > >>> b->h = 1 << (3 + > >>> td->block_structure[block_tile].block_size_idx_y); > >>> > >>> - if (s->s.h.segmentation.feat[seg_id].q_enabled) { > >>> + if (s->s.h.segmentation.enabled && > >>> s->s.h.segmentation.feat[seg_id].q_enabled) { > >>> b->delta_qp = > >>> s->s.h.segmentation.feat[seg_id].q_val; > >>> if (s->s.h.segmentation.absolute_vals) > >>> b->delta_qp -= par->qp; > >>> -- > >>> 2.27.0.383.g050319c2ae-goog > >>> > >>> > >> Dear FFmpeg Developers, > >> > >> Currently ffmpeg doesn't export the block data for VP9 if there is no > >> segmentation. Because it's only used to export QP value. I think it makes > >> more sense to export the block information without segmentation so we could > >> visualize the block structure for VP9 video. > >> > >> Could you please review and merge this patch? Thanks. > >> > >> Best, > >> Yongle > >> > > > > Dear FFmpeg Developers, > > > > Currently ffmpeg doesn't export the block data for VP9 if there is no > > segmentation. Because it's only used to export QP value. I think it makes > > more sense to export the block information without segmentation so we could > > visualize the block structure for VP9 video. > > > > Could you please review and merge this patch? Thanks. > > > > Best, > > Yongle > > > > Hi FFmpeg Developers, > > I noticed that this patch hasn't been reviewed yet. Could someone help me > review it and merge this patch? Thanks. iam not really vp9 maintainer but one thing that is probably a good idea is to add a fate test that covers the code [...]
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c index fd0bab14a2..e700def70e 100644 --- a/libavcodec/vp9.c +++ b/libavcodec/vp9.c @@ -1501,10 +1501,8 @@ static int vp9_export_enc_params(VP9Context *s, VP9Frame *frame) AVVideoEncParams *par; unsigned int tile, nb_blocks = 0; - if (s->s.h.segmentation.enabled) { - for (tile = 0; tile < s->active_tile_cols; tile++) - nb_blocks += s->td[tile].nb_block_structure; - } + for (tile = 0; tile < s->active_tile_cols; tile++) + nb_blocks += s->td[tile].nb_block_structure; par = av_video_enc_params_create_side_data(frame->tf.f, AV_VIDEO_ENC_PARAMS_VP9, nb_blocks); @@ -1536,7 +1534,7 @@ static int vp9_export_enc_params(VP9Context *s, VP9Frame *frame) b->w = 1 << (3 + td->block_structure[block_tile].block_size_idx_x); b->h = 1 << (3 + td->block_structure[block_tile].block_size_idx_y); - if (s->s.h.segmentation.feat[seg_id].q_enabled) { + if (s->s.h.segmentation.enabled && s->s.h.segmentation.feat[seg_id].q_enabled) { b->delta_qp = s->s.h.segmentation.feat[seg_id].q_val; if (s->s.h.segmentation.absolute_vals) b->delta_qp -= par->qp;