diff mbox

[FFmpeg-devel,3/4] lavc/h2645_parse: add h264_nal_unit_name for h264 NAL type.

Message ID 1526015513-17475-3-git-send-email-mypopydev@gmail.com
State Superseded
Headers show

Commit Message

Jun Zhao May 11, 2018, 5:11 a.m. UTC 
Signed-off-by: Jun Zhao <mypopydev@gmail.com>
---
 libavcodec/h2645_parse.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

Comments

Liu Steven May 11, 2018, 7:58 a.m. UTC | #1 
> On May 11, 2018, at 13:11, Jun Zhao <mypopydev@gmail.com> wrote:
> 
> Signed-off-by: Jun Zhao <mypopydev@gmail.com>
> ---
> libavcodec/h2645_parse.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
> index 579b2c9..8d67579 100644
> --- a/libavcodec/h2645_parse.c
> +++ b/libavcodec/h2645_parse.c
> @@ -28,6 +28,7 @@
> 
> #include "bytestream.h"
> #include "hevc.h"
> +#include "h264.h"
> #include "h2645_parse.h"
> 
> int ff_h2645_extract_rbsp(const uint8_t *src, int length,
> @@ -193,6 +194,27 @@ static const char *hevc_nal_unit_name(int nal_type)
>     }
> }
> 
> +static const char *h264_nal_unit_name(int nal_type)
> +{
> +    switch(nal_type) {
> +    case H264_NAL_SLICE           : return "SLICE";
> +    case H264_NAL_DPA             : return "DPA";
> +    case H264_NAL_DPB             : return "DPB";
> +    case H264_NAL_DPC             : return "DPC";
> +    case H264_NAL_IDR_SLICE       : return "IDR_SLICE";
> +    case H264_NAL_SEI             : return "SEI";
> +    case H264_NAL_SPS             : return "SPS";
> +    case H264_NAL_PPS             : return "PPS";
> +    case H264_NAL_AUD             : return "AUD";
> +    case H264_NAL_END_SEQUENCE    : return "END_SEQUENCE";
> +    case H264_NAL_END_STREAM      : return "END_STREAM";
> +    case H264_NAL_FILLER_DATA     : return "FILLER_DATA";
> +    case H264_NAL_SPS_EXT         : return "SPS_EXT";
> +    case H264_NAL_AUXILIARY_SLICE : return "AUXILIARY_SLICE";
> +    default : return "?";
> +    }
> +}
> +
> static int get_bit_length(H2645NAL *nal, int skip_trailing_zeros)
> {
>     int size = nal->size;
> @@ -255,8 +277,8 @@ static int h264_parse_nal_header(H2645NAL *nal, void *logctx)
>     nal->type    = get_bits(gb, 5);
> 
>     av_log(logctx, AV_LOG_DEBUG,
> -           "nal_unit_type: %d, nal_ref_idc: %d\n",
> -           nal->type, nal->ref_idc);
> +           "nal_unit_type: %d(%s), nal_ref_idc: %d\n",
> +           nal->type, h264_nal_unit_name(nal->type), nal->ref_idc);
> 
>     return 1;
> }
> -- 
> 2.7.4
> 

This patch is look good to me, print the type name is simpler than only an value.

Thanks
Steven
Mark Thompson May 11, 2018, 10:19 a.m. UTC | #2 
On 11/05/18 06:11, Jun Zhao wrote:
> Signed-off-by: Jun Zhao <mypopydev@gmail.com>
> ---
>  libavcodec/h2645_parse.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
> index 579b2c9..8d67579 100644
> --- a/libavcodec/h2645_parse.c
> +++ b/libavcodec/h2645_parse.c
> @@ -28,6 +28,7 @@
>  
>  #include "bytestream.h"
>  #include "hevc.h"
> +#include "h264.h"
>  #include "h2645_parse.h"
>  
>  int ff_h2645_extract_rbsp(const uint8_t *src, int length,
> @@ -193,6 +194,27 @@ static const char *hevc_nal_unit_name(int nal_type)
>      }
>  }
>  
> +static const char *h264_nal_unit_name(int nal_type)
> +{
> +    switch(nal_type) {
> +    case H264_NAL_SLICE           : return "SLICE";
> +    case H264_NAL_DPA             : return "DPA";
> +    case H264_NAL_DPB             : return "DPB";
> +    case H264_NAL_DPC             : return "DPC";
> +    case H264_NAL_IDR_SLICE       : return "IDR_SLICE";
> +    case H264_NAL_SEI             : return "SEI";
> +    case H264_NAL_SPS             : return "SPS";
> +    case H264_NAL_PPS             : return "PPS";
> +    case H264_NAL_AUD             : return "AUD";
> +    case H264_NAL_END_SEQUENCE    : return "END_SEQUENCE";
> +    case H264_NAL_END_STREAM      : return "END_STREAM";
> +    case H264_NAL_FILLER_DATA     : return "FILLER_DATA";
> +    case H264_NAL_SPS_EXT         : return "SPS_EXT";
> +    case H264_NAL_AUXILIARY_SLICE : return "AUXILIARY_SLICE";

Unlike H.265 these names aren't defined by the standard, so I think I'd write them normally ("IDR slice", "End of sequence") rather than using the shouty enum names.  Doesn't really matter, though.

> +    default : return "?";
> +    }
> +}
> +
>  static int get_bit_length(H2645NAL *nal, int skip_trailing_zeros)
>  {
>      int size = nal->size;
> @@ -255,8 +277,8 @@ static int h264_parse_nal_header(H2645NAL *nal, void *logctx)
>      nal->type    = get_bits(gb, 5);
>  
>      av_log(logctx, AV_LOG_DEBUG,
> -           "nal_unit_type: %d, nal_ref_idc: %d\n",
> -           nal->type, nal->ref_idc);
> +           "nal_unit_type: %d(%s), nal_ref_idc: %d\n",
> +           nal->type, h264_nal_unit_name(nal->type), nal->ref_idc);
>  
>      return 1;
>  }
> 

LGTM either way.

Thanks,

- Mark
mypopy@gmail.com May 14, 2018, 12:27 a.m. UTC | #3 
2018-05-11 18:19 GMT+08:00 Mark Thompson <sw@jkqxz.net>:
> On 11/05/18 06:11, Jun Zhao wrote:
>> Signed-off-by: Jun Zhao <mypopydev@gmail.com>
>> ---
>>  libavcodec/h2645_parse.c | 26 ++++++++++++++++++++++++--
>>  1 file changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
>> index 579b2c9..8d67579 100644
>> --- a/libavcodec/h2645_parse.c
>> +++ b/libavcodec/h2645_parse.c
>> @@ -28,6 +28,7 @@
>>
>>  #include "bytestream.h"
>>  #include "hevc.h"
>> +#include "h264.h"
>>  #include "h2645_parse.h"
>>
>>  int ff_h2645_extract_rbsp(const uint8_t *src, int length,
>> @@ -193,6 +194,27 @@ static const char *hevc_nal_unit_name(int nal_type)
>>      }
>>  }
>>
>> +static const char *h264_nal_unit_name(int nal_type)
>> +{
>> +    switch(nal_type) {
>> +    case H264_NAL_SLICE           : return "SLICE";
>> +    case H264_NAL_DPA             : return "DPA";
>> +    case H264_NAL_DPB             : return "DPB";
>> +    case H264_NAL_DPC             : return "DPC";
>> +    case H264_NAL_IDR_SLICE       : return "IDR_SLICE";
>> +    case H264_NAL_SEI             : return "SEI";
>> +    case H264_NAL_SPS             : return "SPS";
>> +    case H264_NAL_PPS             : return "PPS";
>> +    case H264_NAL_AUD             : return "AUD";
>> +    case H264_NAL_END_SEQUENCE    : return "END_SEQUENCE";
>> +    case H264_NAL_END_STREAM      : return "END_STREAM";
>> +    case H264_NAL_FILLER_DATA     : return "FILLER_DATA";
>> +    case H264_NAL_SPS_EXT         : return "SPS_EXT";
>> +    case H264_NAL_AUXILIARY_SLICE : return "AUXILIARY_SLICE";
>
> Unlike H.265 these names aren't defined by the standard, so I think I'd write them normally ("IDR slice", "End of sequence") rather than using the shouty enum names.  Doesn't really matter, though.
I will update the code as the comments, everything that makes the code
clearer is worthwhile to do.
>
>> +    default : return "?";
>> +    }
>> +}
>> +
>>  static int get_bit_length(H2645NAL *nal, int skip_trailing_zeros)
>>  {
>>      int size = nal->size;
>> @@ -255,8 +277,8 @@ static int h264_parse_nal_header(H2645NAL *nal, void *logctx)
>>      nal->type    = get_bits(gb, 5);
>>
>>      av_log(logctx, AV_LOG_DEBUG,
>> -           "nal_unit_type: %d, nal_ref_idc: %d\n",
>> -           nal->type, nal->ref_idc);
>> +           "nal_unit_type: %d(%s), nal_ref_idc: %d\n",
>> +           nal->type, h264_nal_unit_name(nal->type), nal->ref_idc);
>>
>>      return 1;
>>  }
>>
>
> LGTM either way.
>
> Thanks,
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff mbox

Patch

diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
index 579b2c9..8d67579 100644
--- a/libavcodec/h2645_parse.c
+++ b/libavcodec/h2645_parse.c
@@ -28,6 +28,7 @@ 
 
 #include "bytestream.h"
 #include "hevc.h"
+#include "h264.h"
 #include "h2645_parse.h"
 
 int ff_h2645_extract_rbsp(const uint8_t *src, int length,
@@ -193,6 +194,27 @@  static const char *hevc_nal_unit_name(int nal_type)
     }
 }
 
+static const char *h264_nal_unit_name(int nal_type)
+{
+    switch(nal_type) {
+    case H264_NAL_SLICE           : return "SLICE";
+    case H264_NAL_DPA             : return "DPA";
+    case H264_NAL_DPB             : return "DPB";
+    case H264_NAL_DPC             : return "DPC";
+    case H264_NAL_IDR_SLICE       : return "IDR_SLICE";
+    case H264_NAL_SEI             : return "SEI";
+    case H264_NAL_SPS             : return "SPS";
+    case H264_NAL_PPS             : return "PPS";
+    case H264_NAL_AUD             : return "AUD";
+    case H264_NAL_END_SEQUENCE    : return "END_SEQUENCE";
+    case H264_NAL_END_STREAM      : return "END_STREAM";
+    case H264_NAL_FILLER_DATA     : return "FILLER_DATA";
+    case H264_NAL_SPS_EXT         : return "SPS_EXT";
+    case H264_NAL_AUXILIARY_SLICE : return "AUXILIARY_SLICE";
+    default : return "?";
+    }
+}
+
 static int get_bit_length(H2645NAL *nal, int skip_trailing_zeros)
 {
     int size = nal->size;
@@ -255,8 +277,8 @@  static int h264_parse_nal_header(H2645NAL *nal, void *logctx)
     nal->type    = get_bits(gb, 5);
 
     av_log(logctx, AV_LOG_DEBUG,
-           "nal_unit_type: %d, nal_ref_idc: %d\n",
-           nal->type, nal->ref_idc);
+           "nal_unit_type: %d(%s), nal_ref_idc: %d\n",
+           nal->type, h264_nal_unit_name(nal->type), nal->ref_idc);
 
     return 1;
 }