[FFmpeg-devel] [PATCH] lavc/hevc_ps: fix crop info for monochrome

James Almer jamrial at gmail.com
Wed Aug 22 20:06:10 EEST 2018


On 8/22/2018 6:40 AM, Zhao Zhili wrote:
> 
> 
> On 2018年08月22日 08:26, Michael Niedermayer wrote:
>> On Mon, Aug 20, 2018 at 10:19:04AM +0800, Zhao Zhili wrote:
>>>
>>> On 2018年08月18日 05:33, Michael Niedermayer wrote:
>>>> On Fri, Aug 17, 2018 at 09:52:57AM +0800, Zhao Zhili wrote:
>>>>> The values of SubWidthC and SubHeightC are 1 in the ITU-T H.265. The
>>>>> current code use the value of 2.
>>>>> ---
>>>>>   libavcodec/hevc_ps.c | 16 ++++++++++++----
>>>>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
>>>>> index fbd9fbf..b56b078 100644
>>>>> --- a/libavcodec/hevc_ps.c
>>>>> +++ b/libavcodec/hevc_ps.c
>>>>> @@ -70,6 +70,14 @@ static const AVRational vui_sar[] = {
>>>>>       {  2,   1 },
>>>>>   };
>>>>> +static const unsigned hevc_sub_width_c[] = {
>>>> uint8_t saves a few bytes
>>>>
>>>> more important, the commit message should mention a ticket or test
>>>> sample
>>>> also a fate test with a _small_ testsample would be usefull. Obviously
>>>> the existing tests do not cover this
>>>>
>>> The bug was found by reading the source code. There is no ticket
>>> related to the bug. I need some time to download the test suite
>>> and figure out how it work. Feel free to add the test if anyone
>>> has a suitable sample.
>> if theres no test sample, creating one would be a good idea so this
>> is tested. Because as is it would be a change that completely untested
> 
> Updated patch and test sample are attached. Please review.
> 
>> thx
>>
>> [...]
>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> 
> 0001-lavc-hevc_ps-fix-crop-info-for-monochrome.patch
> 
> 
> From 74dd822640b359beff6c843143e878fe39fc92c5 Mon Sep 17 00:00:00 2001
> From: Zhao Zhili <quinkblack at foxmail.com>
> Date: Wed, 22 Aug 2018 17:37:15 +0800
> Subject: [PATCH] lavc/hevc_ps: fix crop info for monochrome
> 
> The values of SubWidthC and SubHeightC are 1 in the ITU-T H.265. The
> current code use the value of 2.
> ---
>  libavcodec/hevc_ps.c                | 16 ++++++++++++----
>  tests/fate/hevc.mak                 |  3 +++
>  tests/ref/fate/hevc-monochrome-crop |  6 ++++++
>  3 files changed, 21 insertions(+), 4 deletions(-)
>  create mode 100644 tests/ref/fate/hevc-monochrome-crop
> 
> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> index fbd9fbf..ea984af 100644
> --- a/libavcodec/hevc_ps.c
> +++ b/libavcodec/hevc_ps.c
> @@ -70,6 +70,14 @@ static const AVRational vui_sar[] = {
>      {  2,   1 },
>  };
>  
> +static const uint8_t hevc_sub_width_c[] = {
> +    1, 2, 2, 1
> +};
> +
> +static const uint8_t hevc_sub_height_c[] = {
> +    1, 2, 1, 1
> +};
> +
>  static void remove_pps(HEVCParamSets *s, int id)
>  {
>      if (s->pps_list[id] && s->pps == (const HEVCPPS*)s->pps_list[id]->data)
> @@ -628,8 +636,8 @@ static void decode_vui(GetBitContext *gb, AVCodecContext *avctx,
>          vui->default_display_window_flag = get_bits1(gb);
>  
>      if (vui->default_display_window_flag) {
> -        int vert_mult  = 1 + (sps->chroma_format_idc < 2);
> -        int horiz_mult = 1 + (sps->chroma_format_idc < 3);
> +        int vert_mult  = hevc_sub_height_c[sps->chroma_format_idc];
> +        int horiz_mult = hevc_sub_width_c[sps->chroma_format_idc];
>          vui->def_disp_win.left_offset   = get_ue_golomb_long(gb) * horiz_mult;
>          vui->def_disp_win.right_offset  = get_ue_golomb_long(gb) * horiz_mult;
>          vui->def_disp_win.top_offset    = get_ue_golomb_long(gb) *  vert_mult;
> @@ -923,8 +931,8 @@ int ff_hevc_parse_sps(HEVCSPS *sps, GetBitContext *gb, unsigned int *sps_id,
>          return ret;
>  
>      if (get_bits1(gb)) { // pic_conformance_flag
> -        int vert_mult  = 1 + (sps->chroma_format_idc < 2);
> -        int horiz_mult = 1 + (sps->chroma_format_idc < 3);
> +        int vert_mult  = hevc_sub_height_c[sps->chroma_format_idc];
> +        int horiz_mult = hevc_sub_width_c[sps->chroma_format_idc];
>          sps->pic_conf_win.left_offset   = get_ue_golomb_long(gb) * horiz_mult;
>          sps->pic_conf_win.right_offset  = get_ue_golomb_long(gb) * horiz_mult;
>          sps->pic_conf_win.top_offset    = get_ue_golomb_long(gb) *  vert_mult;
> diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
> index 184349e..9c288fc 100644
> --- a/tests/fate/hevc.mak
> +++ b/tests/fate/hevc.mak
> @@ -250,6 +250,9 @@ FATE_HEVC-$(call DEMDEC, MOV, HEVC) += fate-hevc-extradata-reload
>  
>  fate-hevc-extradata-reload: CMD = framemd5 -i $(TARGET_SAMPLES)/hevc/extradata-reload-multi-stsd.mov -sws_flags bitexact
>  
> +fate-hevc-monochrome-crop: CMD = ffprobe$(PROGSSUF)$(EXESUF) -show_entries stream=width,height,coded_width,coded_height $(TARGET_SAMPLES)/hevc/hevc-monochrome.hevc

This should have called run() before ffprobe. It fails otherwise as
$(TARGET_PATH) is not added.

Changed it to use the probeframes() function anyway so it may also probe
the reported size of the frame in the stream, removed coded_width/height
as those are deprecated and will be removed in an upcoming major version
bump, and applied it.

Thanks.

> +FATE_HEVC_FFPROBE-$(call DEMDEC, HEVC, HEVC) += fate-hevc-monochrome-crop
> +
>  FATE_SAMPLES_AVCONV += $(FATE_HEVC-yes)
>  FATE_SAMPLES_FFPROBE += $(FATE_HEVC_FFPROBE-yes)
>  
> diff --git a/tests/ref/fate/hevc-monochrome-crop b/tests/ref/fate/hevc-monochrome-crop
> new file mode 100644
> index 0000000..531bfc8
> --- /dev/null
> +++ b/tests/ref/fate/hevc-monochrome-crop
> @@ -0,0 +1,6 @@
> +[STREAM]
> +width=384
> +height=240
> +coded_width=384
> +coded_height=256
> +[/STREAM]
> -- 2.9.5
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 



More information about the ffmpeg-devel mailing list