[FFmpeg-devel,2/2] lavc/hevc_ps: use skip_bits instead of get_bits when skip bits.

Submitted by Jun Zhao on July 7, 2018, 5:48 a.m.

Details

Message ID 1530942529-17402-2-git-send-email-mypopydev@gmail.com
State New
Headers show

Commit Message

Jun Zhao July 7, 2018, 5:48 a.m.
use skip_bits when want to skip some bits.

Signed-off-by: Jun Zhao <mypopydev@gmail.com>
---
 libavcodec/hevc_ps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Carl Eugen Hoyos July 7, 2018, 10:25 a.m.
2018-07-07 7:48 GMT+02:00, Jun Zhao <mypopydev@gmail.com>:
> use skip_bits when want to skip some bits.
>
> Signed-off-by: Jun Zhao <mypopydev@gmail.com>
> ---
>  libavcodec/hevc_ps.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> index bc5406b..278b928 100644
> --- a/libavcodec/hevc_ps.c
> +++ b/libavcodec/hevc_ps.c
> @@ -1679,7 +1679,7 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb,
> AVCodecContext *avctx,
>
>      if (get_bits1(gb)) { // pps_extension_present_flag
>          int pps_range_extensions_flag = get_bits1(gb);
> -        /* int pps_extension_7bits = */ get_bits(gb, 7);
> +        skip_bits(gb, 7);

This seems to make the code less readable, no?

Carl Eugen
mypopy@gmail.com July 9, 2018, 12:16 a.m.
On Sat, Jul 7, 2018 at 6:32 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> 2018-07-07 7:48 GMT+02:00, Jun Zhao <mypopydev@gmail.com>:
> > use skip_bits when want to skip some bits.
> >
> > Signed-off-by: Jun Zhao <mypopydev@gmail.com>
> > ---
> >  libavcodec/hevc_ps.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> > index bc5406b..278b928 100644
> > --- a/libavcodec/hevc_ps.c
> > +++ b/libavcodec/hevc_ps.c
> > @@ -1679,7 +1679,7 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb,
> > AVCodecContext *avctx,
> >
> >      if (get_bits1(gb)) { // pps_extension_present_flag
> >          int pps_range_extensions_flag = get_bits1(gb);
> > -        /* int pps_extension_7bits = */ get_bits(gb, 7);
> > +        skip_bits(gb, 7);
>
> This seems to make the code less readable, no?

You means keep old style? But I think skip_bits more clear in this
case, maybe I need to add some comment?

>
>
> Carl Eugen
James Almer July 9, 2018, 12:35 a.m.
On 7/8/2018 9:16 PM, mypopy@gmail.com wrote:
> On Sat, Jul 7, 2018 at 6:32 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>
>> 2018-07-07 7:48 GMT+02:00, Jun Zhao <mypopydev@gmail.com>:
>>> use skip_bits when want to skip some bits.
>>>
>>> Signed-off-by: Jun Zhao <mypopydev@gmail.com>
>>> ---
>>>  libavcodec/hevc_ps.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
>>> index bc5406b..278b928 100644
>>> --- a/libavcodec/hevc_ps.c
>>> +++ b/libavcodec/hevc_ps.c
>>> @@ -1679,7 +1679,7 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb,
>>> AVCodecContext *avctx,
>>>
>>>      if (get_bits1(gb)) { // pps_extension_present_flag
>>>          int pps_range_extensions_flag = get_bits1(gb);
>>> -        /* int pps_extension_7bits = */ get_bits(gb, 7);
>>> +        skip_bits(gb, 7);
>>
>> This seems to make the code less readable, no?
> 
> You means keep old style? But I think skip_bits more clear in this
> case, maybe I need to add some comment?

He probably means leaving the comment about what the skipped field/s is.
mypopy@gmail.com July 9, 2018, 1:23 a.m.
On Mon, Jul 9, 2018 at 8:41 AM James Almer <jamrial@gmail.com> wrote:
>
> On 7/8/2018 9:16 PM, mypopy@gmail.com wrote:
> > On Sat, Jul 7, 2018 at 6:32 PM Carl Eugen Hoyos <ceffmpeg@gmail.com>
wrote:
> >>
> >> 2018-07-07 7:48 GMT+02:00, Jun Zhao <mypopydev@gmail.com>:
> >>> use skip_bits when want to skip some bits.
> >>>
> >>> Signed-off-by: Jun Zhao <mypopydev@gmail.com>
> >>> ---
> >>>  libavcodec/hevc_ps.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> >>> index bc5406b..278b928 100644
> >>> --- a/libavcodec/hevc_ps.c
> >>> +++ b/libavcodec/hevc_ps.c
> >>> @@ -1679,7 +1679,7 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb,
> >>> AVCodecContext *avctx,
> >>>
> >>>      if (get_bits1(gb)) { // pps_extension_present_flag
> >>>          int pps_range_extensions_flag = get_bits1(gb);
> >>> -        /* int pps_extension_7bits = */ get_bits(gb, 7);
> >>> +        skip_bits(gb, 7);
> >>
> >> This seems to make the code less readable, no?
> >
> > You means keep old style? But I think skip_bits more clear in this
> > case, maybe I need to add some comment?
>
> He probably means leaving the comment about what the skipped field/s is.
I see, will keep the comment about the skipped fields
​.​

Patch hide | download patch | download mbox

diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
index bc5406b..278b928 100644
--- a/libavcodec/hevc_ps.c
+++ b/libavcodec/hevc_ps.c
@@ -1679,7 +1679,7 @@  int ff_hevc_decode_nal_pps(GetBitContext *gb, AVCodecContext *avctx,
 
     if (get_bits1(gb)) { // pps_extension_present_flag
         int pps_range_extensions_flag = get_bits1(gb);
-        /* int pps_extension_7bits = */ get_bits(gb, 7);
+        skip_bits(gb, 7);
         if (sps->ptl.general_ptl.profile_idc == FF_PROFILE_HEVC_REXT && pps_range_extensions_flag) {
             if ((ret = pps_range_extensions(gb, avctx, pps, sps)) < 0)
                 goto err;