[FFmpeg-devel] [PATCH 1/9] cbs_av1: fix incorrect data type

[Wang, Fei W]

On Fri, 2021-07-02 at 08:29 -0300, James Almer wrote:
> On 7/2/2021 2:55 AM, Wang, Fei W wrote:
> > On Thu, 2021-07-01 at 09:41 -0300, James Almer wrote:
> > > On 6/17/2021 3:10 AM, Fei Wang wrote:
> > > > shifted_order_hints is computed by data with int plus data with
> > > > int.
> > > > Switch to int8_t may lose its precision.
> > > > 
> > > > Signed-off-by: Fei Wang <fei.w.wang at intel.com>
> > > > ---
> > > >    libavcodec/cbs_av1_syntax_template.c | 2 +-
> > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/libavcodec/cbs_av1_syntax_template.c
> > > > b/libavcodec/cbs_av1_syntax_template.c
> > > > index 6fe6e9a4f3..956d45e132 100644
> > > > --- a/libavcodec/cbs_av1_syntax_template.c
> > > > +++ b/libavcodec/cbs_av1_syntax_template.c
> > > > @@ -355,7 +355,7 @@ static int
> > > > FUNC(set_frame_refs)(CodedBitstreamContext *ctx, RWContext *rw,
> > > >            AV1_REF_FRAME_ALTREF2, AV1_REF_FRAME_ALTREF
> > > >        };
> > > >        int8_t ref_frame_idx[AV1_REFS_PER_FRAME],
> > > > used_frame[AV1_NUM_REF_FRAMES];
> > > > -    int8_t shifted_order_hints[AV1_NUM_REF_FRAMES];
> > > > +    int shifted_order_hints[AV1_NUM_REF_FRAMES];
> > > 
> > > Would int16_t be enough? If so, use that.
> > 
> > int16_t can fixed my clip. But as I mentioned in commit message,
> > this
> > variable is get with int plus int, switch to int16_t may still has
> > potential threat.
> 
> It wont since seq->order_hint_bits_minus_1 has a range of 0-7,
> meaning 
> cur_frame_hint can be at most 128. Similar situation for the return 
> value of cbs_av1_get_relative_dist().

That's make sense. Thanks James. I will submit V2 to use int16_t. 

Thanks
Fei

> 
> > 
> > Also when shifted_order_hints is called, it turns back to int
> > again:
> > 
> >      int hint = shifted_order_hints[i];
> 
> For arrays we prefer fixed size types, whereas for scalars native
> sizes 
> are better.
> 
> > 
> > Thanks
> > Fei
> > 
> > > 
> > > LGTM either way.
> > > 
> > > >        int cur_frame_hint, latest_order_hint,
> > > > earliest_order_hint,
> > > > ref;
> > > >        int i, j;
> > > >    
> > > > 
> > > 
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > 
> > > To unsubscribe, visit link above, or email
> > > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> > 
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> > 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".