| Message ID | CAB0OVGpy2N2ixnOea5G0_qDXjq3o0ViewioCrfcu9H_N89hNBw@mail.gmail.com |
|---|---|
| State | Withdrawn |
| Headers | show |
On Thu, Oct 05, 2017 at 08:31:39AM +0200, Carl Eugen Hoyos wrote: > Hi! > > Attached patch fixes ticket #6717. > > Please comment, Carl Eugen > h264_cabac.c | 2 +- > h264_mb.c | 2 +- > h264_slice.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > 9e712a4dbc032aa844d8693e699f105d99e4b3d5 0001-lavc-h264-Only-check-x264_build-if-it-was-set.patch > From 3315fa024958246685b2431c2605e867f651ad5b Mon Sep 17 00:00:00 2001 > From: Carl Eugen Hoyos <ceffmpeg@gmail.com> > Date: Thu, 5 Oct 2017 08:29:27 +0200 > Subject: [PATCH] lavc/h264: Only check x264_build if it was set. > > Fixes ticket #6718. 6717 or 6718 ? also the default x264_build is -1, that should already be larger than the unsigned thresholds but maybe iam missing something [...]
On Thu, Oct 5, 2017 at 8:31 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > Hi! > > Attached patch fixes ticket #6717. > > Please comment, Carl Eugen Signed numbers are converted to unsigned when compared to unsigned numbers which means -1 becomes UINT_MAX so this patch shouldn't actually change anything. #6717 is probably unfixable without breaking something else. Old x264 versions produced non-spec-compliant output in the case of 4:4:4 + cabac + 8x8dct, and old libavcodec versions had the same bug on the decoder side which is now fixed to handle spec-compliant files. libavcodec supports decoding older broken files when it detects them in order to avoid breaking backwards-compatibility, but they can't be detected as such if someone removed the x264 SEI. tl;dr; #6717 is one of the reasons why messing with SEI messages is generally a bad idea unless you know what you're doing.
On Fri, Oct 6, 2017 at 6:54 PM, Henrik Gramner <henrik@gramner.com> wrote: > On Thu, Oct 5, 2017 at 8:31 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> Hi! >> >> Attached patch fixes ticket #6717. >> >> Please comment, Carl Eugen > > Signed numbers are converted to unsigned when compared to unsigned > numbers which means -1 becomes UINT_MAX so this patch shouldn't > actually change anything. > > #6717 is probably unfixable without breaking something else. Old x264 > versions produced non-spec-compliant output in the case of 4:4:4 + > cabac + 8x8dct, and old libavcodec versions had the same bug on the > decoder side which is now fixed to handle spec-compliant files. > > libavcodec supports decoding older broken files when it detects them > in order to avoid breaking backwards-compatibility, but they can't be > detected as such if someone removed the x264 SEI. > > tl;dr; #6717 is one of the reasons why messing with SEI messages is > generally a bad idea unless you know what you're doing. Its unfortunate that the CABAC change causes entire bitstream destruction. I don't suppose its in any way feasible to somehow detect the difference somehow, or re-try if it failed? - Hendrik
2017-10-06 18:54 GMT+02:00 Henrik Gramner <henrik@gramner.com>: > On Thu, Oct 5, 2017 at 8:31 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> Hi! >> >> Attached patch fixes ticket #6717. >> >> Please comment, Carl Eugen > > Signed numbers are converted to unsigned when compared to unsigned > numbers which means -1 becomes UINT_MAX so this patch shouldn't > actually change anything. Thank you both for pointing this out! > #6717 is probably unfixable without breaking something else. I believe an option to force the work-around should at least be discussed. Do you already know how exactly the SEI was removed? Can this happen by accident? Thank you, Carl Eugen
On Fri, Oct 06, 2017 at 08:19:43PM +0200, Carl Eugen Hoyos wrote: > 2017-10-06 18:54 GMT+02:00 Henrik Gramner <henrik@gramner.com>: > > On Thu, Oct 5, 2017 at 8:31 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> Hi! > >> > >> Attached patch fixes ticket #6717. > >> > >> Please comment, Carl Eugen > > > > Signed numbers are converted to unsigned when compared to unsigned > > numbers which means -1 becomes UINT_MAX so this patch shouldn't > > actually change anything. > > Thank you both for pointing this out! > > > #6717 is probably unfixable without breaking something else. > > I believe an option to force the work-around should at least be > discussed. If theres need to add an option then a value in workaround_bugs can be added. This field is used in other decoders for the same purpose Or some option to set x264_build could be added. This may be better [...]
From 3315fa024958246685b2431c2605e867f651ad5b Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <ceffmpeg@gmail.com> Date: Thu, 5 Oct 2017 08:29:27 +0200 Subject: [PATCH] lavc/h264: Only check x264_build if it was set. Fixes ticket #6718. --- libavcodec/h264_cabac.c | 2 +- libavcodec/h264_mb.c | 2 +- libavcodec/h264_slice.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libavcodec/h264_cabac.c b/libavcodec/h264_cabac.c index 3458346..7d9a7e9 100644 --- a/libavcodec/h264_cabac.c +++ b/libavcodec/h264_cabac.c @@ -2347,7 +2347,7 @@ decode_intra_mb: if (CHROMA444(h) && IS_8x8DCT(mb_type)){ int i; uint8_t *nnz_cache = sl->non_zero_count_cache; - if (h->sei.unregistered.x264_build < 151U) { + if (h->sei.unregistered.x264_build > 0 && h->sei.unregistered.x264_build < 151U) { for (i = 0; i < 2; i++){ if (sl->left_type[LEFT(i)] && !IS_8x8DCT(sl->left_type[LEFT(i)])) { nnz_cache[3+8* 1 + 2*8*i]= diff --git a/libavcodec/h264_mb.c b/libavcodec/h264_mb.c index cb9fe85..55a8d85 100644 --- a/libavcodec/h264_mb.c +++ b/libavcodec/h264_mb.c @@ -637,7 +637,7 @@ static av_always_inline void hl_decode_mb_predict_luma(const H264Context *h, uint8_t *const ptr = dest_y + block_offset[i]; const int dir = sl->intra4x4_pred_mode_cache[scan8[i]]; if (transform_bypass && h->ps.sps->profile_idc == 244 && dir <= 1) { - if (h->sei.unregistered.x264_build < 151U) { + if (h->sei.unregistered.x264_build > 0 && h->sei.unregistered.x264_build < 151U) { h->hpc.pred8x8l_add[dir](ptr, sl->mb + (i * 16 + p * 256 << pixel_shift), linesize); } else h->hpc.pred8x8l_filter_add[dir](ptr, sl->mb + (i * 16 + p * 256 << pixel_shift), diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c index 2577edd..e69c2a0 100644 --- a/libavcodec/h264_slice.c +++ b/libavcodec/h264_slice.c @@ -921,7 +921,7 @@ static int h264_slice_header_init(H264Context *h) if (sps->timing_info_present_flag) { int64_t den = sps->time_scale; - if (h->sei.unregistered.x264_build < 44U) + if (h->sei.unregistered.x264_build > 0 && h->sei.unregistered.x264_build < 44U) den *= 2; av_reduce(&h->avctx->framerate.den, &h->avctx->framerate.num, sps->num_units_in_tick * h->avctx->ticks_per_frame, den, 1 << 30); -- 1.7.10.4