Message ID | CAB0OVGpQdn1G8=VNju6_WbMw65gLc3VOApK=5gKP9BgCN0=9Hw@mail.gmail.com |
---|---|
State | Superseded |
Headers | show |
On 04/12/17 00:09, Carl Eugen Hoyos wrote: > Hi! > > Attached patch fixes ticket #6717, files without sei can be produced > with remuxing and seeking, even if this is a (separate) bug, such > files exist in the wild. > > Please comment, Carl Eugen File are currently fixable by remuxing with: -bsf:v 'h264_metadata=sei_user_data=dc45e9bde6d948b7962cd820d923eeef+x264 - core 150' Unfortunately that can't be applied directly when decoding, because -bsf isn't available as an input option. I don't really like the idea of adding the option because it does break correct files, but I guess it probably ends up being the pragmatic option for decoding. Still, I think it might be sensible to at least add a big warning telling the user they're doing something stupid if this option is set and it finds x264_build is >= 151? - Mark
2017-12-04 1:56 GMT+01:00 Mark Thompson <sw@jkqxz.net>: > On 04/12/17 00:09, Carl Eugen Hoyos wrote: >> Attached patch fixes ticket #6717, files without sei can be produced >> with remuxing and seeking, even if this is a (separate) bug, such >> files exist in the wild. > File are currently fixable by remuxing with: > > -bsf:v 'h264_metadata=sei_user_data=dc45e9bde6d948b7962cd820d923eeef+x264 - core 150' Thank you! > Unfortunately that can't be applied directly when decoding, > because -bsf isn't available as an input option. > I don't really like the idea of adding the option because it does > break correct files, but I guess it probably ends up being the > pragmatic option for decoding. Sorry, I don't understand: Do you mean if the user specifies the work-around, it will break files encoded with current x264? Isn't that the whole point of this (and every other) work-around? > Still, I think it might be sensible to at least add a big warning > telling the user they're doing something stupid if this option > is set and it finds x264_build is >= 151? I believe you know that I am a big fan of warnings but printing a warning when a user explicitely specifies an option? We don't warn when an experimental encoder is used (not even when we know it will segfault) and I don't think it is necessary. Thank you, Carl Eugen
On Mon, 4 Dec 2017 00:56:36 +0000 Mark Thompson <sw@jkqxz.net> wrote: > On 04/12/17 00:09, Carl Eugen Hoyos wrote: > > Hi! > > > > Attached patch fixes ticket #6717, files without sei can be produced > > with remuxing and seeking, even if this is a (separate) bug, such > > files exist in the wild. > > > > Please comment, Carl Eugen > > File are currently fixable by remuxing with: > > -bsf:v 'h264_metadata=sei_user_data=dc45e9bde6d948b7962cd820d923eeef+x264 - core 150' Is that only in the first packet? Then it doesn't work if you start playback at a later frame, unless you somehow decode the first packet anyway. IMO libavcodec should detect buggy files without relying on version info.
On 04/12/17 02:21, Carl Eugen Hoyos wrote: > 2017-12-04 1:56 GMT+01:00 Mark Thompson <sw@jkqxz.net>: >> On 04/12/17 00:09, Carl Eugen Hoyos wrote: > >>> Attached patch fixes ticket #6717, files without sei can be produced >>> with remuxing and seeking, even if this is a (separate) bug, such >>> files exist in the wild. > >> File are currently fixable by remuxing with: >> >> -bsf:v 'h264_metadata=sei_user_data=dc45e9bde6d948b7962cd820d923eeef+x264 - core 150' > > Thank you! > >> Unfortunately that can't be applied directly when decoding, >> because -bsf isn't available as an input option. > >> I don't really like the idea of adding the option because it does >> break correct files, but I guess it probably ends up being the >> pragmatic option for decoding. > > Sorry, I don't understand: Do you mean if the user specifies > the work-around, it will break files encoded with current > x264? Isn't that the whole point of this (and every other) > work-around? > >> Still, I think it might be sensible to at least add a big warning >> telling the user they're doing something stupid if this option >> is set and it finds x264_build is >= 151? > > I believe you know that I am a big fan of warnings but printing > a warning when a user explicitely specifies an option? We > don't warn when an experimental encoder is used (not even > when we know it will segfault) and I don't think it is necessary. There are three cases here: i) The file contains the SEI for x264 build < 151. ii) The file contains the SEI for x264 build >= 151. iii) The file does not contain any x264 SEI, so we can't tell whether it is broken. I'm suggesting printing a warning only in case (ii), where we know that applying the workaround is almost certainly wrong but will of course do so anyway. (Feel free to ignore this suggestion.) - Mark
On 04/12/17 21:45, wm4 wrote: > On Mon, 4 Dec 2017 00:56:36 +0000 > Mark Thompson <sw@jkqxz.net> wrote: > >> On 04/12/17 00:09, Carl Eugen Hoyos wrote: >>> Hi! >>> >>> Attached patch fixes ticket #6717, files without sei can be produced >>> with remuxing and seeking, even if this is a (separate) bug, such >>> files exist in the wild. >>> >>> Please comment, Carl Eugen >> >> File are currently fixable by remuxing with: >> >> -bsf:v 'h264_metadata=sei_user_data=dc45e9bde6d948b7962cd820d923eeef+x264 - core 150' > > Is that only in the first packet? Then it doesn't work if you start > playback at a later frame, unless you somehow decode the first packet > anyway. It adds it to every access unit containing an SPS, which will cover this in most cases. (Though not all, I admit - SEI not being allowed in AVCC extradata means that if the parameter sets are only there then you are stuck.) > IMO libavcodec should detect buggy files without relying on version > info. Unfortunately that is rather difficult in this case - pretty much the only method to have it happen automatically would be to try decoding the whole frame again in the other mode if there is a failure during entropy decoding, which doesn't sound very fun to write. - Mark
From c3ce0a75f13663958034a70fb4a982671532d269 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <ceffmpeg@gmail.com> Date: Mon, 4 Dec 2017 01:05:57 +0100 Subject: [PATCH] lavc: Allow forcing work-around for old x264 cabac 8x8 4:4:4 bug. Fixes ticket #6717. --- libavcodec/avcodec.h | 1 + libavcodec/h264_cabac.c | 5 ++++- libavcodec/options_table.h | 1 + libavcodec/version.h | 2 +- 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index 5db6a81..bae8812 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -2516,6 +2516,7 @@ typedef struct AVCodecContext { */ int workaround_bugs; #define FF_BUG_AUTODETECT 1 ///< autodetection +#define FF_BUG_X264_CAB_8x8_444 2 #define FF_BUG_XVID_ILACE 4 #define FF_BUG_UMP4 8 #define FF_BUG_NO_PADDING 16 diff --git a/libavcodec/h264_cabac.c b/libavcodec/h264_cabac.c index ec5fc74..8b2973a 100644 --- a/libavcodec/h264_cabac.c +++ b/libavcodec/h264_cabac.c @@ -1919,6 +1919,9 @@ int ff_h264_decode_mb_cabac(const H264Context *h, H264SliceContext *sl) int dct8x8_allowed = h->ps.pps->transform_8x8_mode; const int decode_chroma = sps->chroma_format_idc == 1 || sps->chroma_format_idc == 2; const int pixel_shift = h->pixel_shift; + AVCodecContext *avctx = h->avctx; + if (h->x264_build < 151U) + avctx->workaround_bugs |= FF_BUG_X264_CAB_8x8_444; mb_xy = sl->mb_xy = sl->mb_x + sl->mb_y*h->mb_stride; @@ -2347,7 +2350,7 @@ decode_intra_mb: if (CHROMA444(h) && IS_8x8DCT(mb_type)){ int i; uint8_t *nnz_cache = sl->non_zero_count_cache; - if (h->x264_build < 151U) { + if (avctx->workaround_bugs & FF_BUG_X264_CAB_8x8_444) { 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/options_table.h b/libavcodec/options_table.h index d89f58d..ed35bec 100644 --- a/libavcodec/options_table.h +++ b/libavcodec/options_table.h @@ -112,6 +112,7 @@ static const AVOption avcodec_options[] = { {"bug", "work around not autodetected encoder bugs", OFFSET(workaround_bugs), AV_OPT_TYPE_FLAGS, {.i64 = FF_BUG_AUTODETECT }, INT_MIN, INT_MAX, V|D, "bug"}, {"autodetect", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_BUG_AUTODETECT }, INT_MIN, INT_MAX, V|D, "bug"}, {"xvid_ilace", "Xvid interlacing bug (autodetected if FOURCC == XVIX)", 0, AV_OPT_TYPE_CONST, {.i64 = FF_BUG_XVID_ILACE }, INT_MIN, INT_MAX, V|D, "bug"}, +{"x264_cab_8x8_444", "x264 cabac 8x8 4:4:4 encoding bug (autodetected if possible)", 0, AV_OPT_TYPE_CONST, {.i64 = FF_BUG_X264_CAB_8x8_444 }, INT_MIN, INT_MAX, V|D, "bug"}, {"ump4", "(autodetected if FOURCC == UMP4)", 0, AV_OPT_TYPE_CONST, {.i64 = FF_BUG_UMP4 }, INT_MIN, INT_MAX, V|D, "bug"}, {"no_padding", "padding bug (autodetected)", 0, AV_OPT_TYPE_CONST, {.i64 = FF_BUG_NO_PADDING }, INT_MIN, INT_MAX, V|D, "bug"}, {"amv", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_BUG_AMV }, INT_MIN, INT_MAX, V|D, "bug"}, diff --git a/libavcodec/version.h b/libavcodec/version.h index d67b689..3b5c300 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -29,7 +29,7 @@ #define LIBAVCODEC_VERSION_MAJOR 58 #define LIBAVCODEC_VERSION_MINOR 6 -#define LIBAVCODEC_VERSION_MICRO 102 +#define LIBAVCODEC_VERSION_MICRO 103 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ LIBAVCODEC_VERSION_MINOR, \ -- 1.7.10.4