Opened 7 months ago

Closed 5 months ago

Last modified 5 months ago

#6356 closed defect (fixed)

implementation of "scaling_list_data" function (hevc_ps.c) does not conform to HEVC standard

Reported by: ffmpeg_jhwu Owned by:
Priority: important Component: avcodec
Version: git-master Keywords: hevc regression
Cc: Blocked By:
Blocking: Reproduced by developer: yes
Analyzed by developer: no

Description

Current implementation:

if (matrix_id < delta) {

av_log(avctx, AV_LOG_ERROR, "Invalid delta in scaling list data: %d.\n", delta);
return AVERROR_INVALIDDATA;

}

memcpy(sl->sl[size_id][matrix_id],

sl->sl[size_id][matrix_id - delta],
size_id > 0 ? 64 : 16);

if (size_id > 1)

sl->sl_dc[size_id - 2][matrix_id] = sl->sl_dc[size_id - 2][matrix_id - delta];

Suggested implementation:

refMatrixId = matrix_id - delta*(size_id ==3?3:1);
if (refMatrixId < 0) {

av_log(avctx, AV_LOG_ERROR, "Invalid delta in scaling list data: %d.\n", delta);
return AVERROR_INVALIDDATA;

}

memcpy(sl->sl[size_id][matrix_id],

sl->sl[size_id][refMatrixId],
size_id > 0 ? 64 : 16);

if (size_id > 1)

sl->sl_dc[size_id - 2][matrix_id] = sl->sl_dc[size_id - 2][refMatrixId];

Attachments (1)

sample.bin (46.1 KB) - added by ffmpeg_jhwu 7 months ago.
sample HEVC bitstream

Download all attachments as: .zip

Change History (11)

comment:1 Changed 7 months ago by jamrial

If you think this change is correct then please send a patch to ffmpeg-devel mailing list for review.
See https://ffmpeg.org/developer.html#Submitting-patches for details.

Thanks.

comment:2 Changed 7 months ago by cehoyos

  • Keywords hevc added
  • Priority changed from critical to normal

If you don't want to send a patch - made with git format-patch - then please provide a sample that allows to reproduce the issue you see.

Changed 7 months ago by ffmpeg_jhwu

sample HEVC bitstream

comment:3 Changed 7 months ago by ffmpeg_jhwu

Thank you very much for your quick reply.

I attached the sample HEVC bitstream. For the attached sample bitstream, decoded YUV by ffmpeg is different from decoded YUV by HM 16.0 decoder.This is because the parsing of scaling list in the sample bitstream is wrong for current ffmpeg software. Please let me know if you need more information. Thanks.

comment:4 Changed 7 months ago by cehoyos

  • Reproduced by developer set
  • Status changed from new to open
  • Version changed from unspecified to git-master

comment:5 Changed 7 months ago by cehoyos

  • Keywords regression added
  • Priority changed from normal to important

Regression since 627c044f50da3046809314f7cc742b8a10cf26a1 (analyzed by Mark Thompson).

comment:6 follow-up: Changed 6 months ago by jkqxz

Would it be acceptable for this sample to be added to the ffmpeg regression test suite?

(Patch at https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-April/210649.html , but would like a regression test.)

comment:7 Changed 5 months ago by jkqxz

  • Resolution set to fixed
  • Status changed from open to closed

Fixed in 88a2e4504d1820e717730ff5f5dc0cf4d90cdd6e. A test-usable sample would still be welcome.

comment:8 Changed 5 months ago by cehoyos

Please use the sample attached to this ticket (cut it if possible).

comment:9 in reply to: ↑ 6 Changed 5 months ago by jamrial

Replying to jkqxz:

Would it be acceptable for this sample to be added to the ffmpeg regression test suite?

The sample.bin file attached here seems to be the same footage as a bunch of the hevc-conformance samples in the fate suite (see AMVP_C_Samsung), so i'd say it can be added.

Replying to cehoyos:

Please use the sample attached to this ticket (cut it if possible).

It weighs 46k and is 5 frames long, so no need to cut it.

comment:10 Changed 5 months ago by cehoyos

True, sorry!

Note: See TracTickets for help on using tickets.