Opened 9 years ago
Closed 9 years ago
#4876 closed defect (fixed)
DNx100 (DNxHD CID 1260) decode produces partially corrupted picture
Reported by: | Jeremy | Owned by: | |
---|---|---|---|
Priority: | normal | Component: | avcodec |
Version: | git-master | Keywords: | dnxhd |
Cc: | Christophe, projectsymphony@gmail.com | Blocked By: | |
Blocking: | Reproduced by developer: | yes | |
Analyzed by developer: | yes |
Description
Summary of the bug:
Decoding a file for this format, support being added recently by Vittorio Giovara in d68705c9, results in some blocks not being decoded properly as per the attached screenshot.
How to reproduce:
% ffmpeg -i DNxHD100_cid1260_example.mov /tmp/out.mov ffmpeg version N-75480-ge859a3c Copyright (c) 2000-2015 the FFmpeg developers built with gcc 4.7 (Debian 4.7.2-5) configuration: --enable-gpl --enable-version3 --enable-gnutls libavutil 55. 2.100 / 55. 2.100 libavcodec 57. 3.100 / 57. 3.100 libavformat 57. 2.100 / 57. 2.100 libavdevice 57. 0.100 / 57. 0.100 libavfilter 6. 7.100 / 6. 7.100 libswscale 4. 0.100 / 4. 0.100 libswresample 2. 0.100 / 2. 0.100 libpostproc 54. 0.100 / 54. 0.100 Input #0, mov,mp4,m4a,3gp,3g2,mj2, from '/video/jbj/DNxHD100_cid1260_example.mov': Metadata: major_brand : qt minor_version : 512 compatible_brands: qt encoder : Lavf55.48.100 Duration: 00:00:00.37, start: 0.000000, bitrate: 99926 kb/s Stream #0:0(eng): Video: dnxhd (AVdn / 0x6E645641), yuv422p(pc), 1440x1080 [SAR 4:3 DAR 16:9], 100169 kb/s, 29.97 fps, 29.97 tbr, 30k tbn, 30k tbc (default) Metadata: handler_name : DataHandler Output #0, mov, to '/tmp/out.mov': Metadata: major_brand : qt minor_version : 512 compatible_brands: qt encoder : Lavf57.2.100 Stream #0:0(eng): Video: mpeg4 (mp4v / 0x7634706D), yuv420p, 1440x1080 [SAR 4:3 DAR 16:9], q=2-31, 200 kb/s, 29.97 fps, 30k tbn, 29.97 tbc (default) Metadata: handler_name : DataHandler encoder : Lavc57.3.100 mpeg4 Stream mapping: Stream #0:0 -> #0:0 (dnxhd (native) -> mpeg4 (native)) Press [q] to stop, [?] for help frame= 11 fps=0.0 q=31.0 Lsize= 2854kB time=00:00:00.36 bitrate=63688.9kbits/s video:2853kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 0.034509%
Attached files also available at
http://jeremy.publication.org.uk/DNxHD100_cid1260_example.mov (4.4MB source file)
http://jeremy.publication.org.uk/dnx_screenshot.jpg (output screenshot)
It was suggested this may be related to other DNxHD DCT issues https://bugzilla.libav.org/show_bug.cgi?id=823 (albeit related to encoding)
Attachments (9)
Change History (23)
by , 9 years ago
Attachment: | dnx_screenshot.jpg added |
---|
comment:1 by , 9 years ago
Keywords: | dnx100 removed |
---|---|
Reproduced by developer: | set |
Status: | new → open |
by , 9 years ago
Attachment: | DNxHD100_cid1260_example_cut.mov added |
---|
follow-up: 3 comment:2 by , 9 years ago
Does anyone have the specs? qscale on 11 bits for 8 bits content is overkill. I bet some of those bits are not what they are used for.
EDIT: attached a patch that seems to fix the issue, but this can break something else.
by , 9 years ago
Attachment: | dnx100_closeups.png added |
---|
Could that be a bit to indicate that the macroblocks should be decoded separately, then interleaved? More obvious from the end of the original clip than the truncated one.
follow-up: 4 comment:3 by , 9 years ago
Cc: | added |
---|
Replying to kurosu:
Does anyone have the specs? qscale on 11 bits for 8 bits content is overkill. I bet some of those bits are not what they are used for.
But it would not be overkill for 10bit iiuc?
diff --git a/libavcodec/dnxhddec.c b/libavcodec/dnxhddec.c index 1034d89..cb4a7db 100644 --- a/libavcodec/dnxhddec.c +++ b/libavcodec/dnxhddec.c @@ -350,7 +350,12 @@ static int dnxhd_decode_macroblock(DNXHDContext *ctx, AVFrame *frame, int dct_y_offset, dct_x_offset; int qscale, i; + if (ctx->bit_depth == 8) { + skip_bits1(&ctx->gb); + qscale = get_bits(&ctx->gb, 10); + } else { qscale = get_bits(&ctx->gb, 11); + } skip_bits1(&ctx->gb); if (qscale != ctx->last_qscale) {
EDIT: attached a patch that seems to fix the issue, but this can break something else.
After some testing, we will only find out if you push;-)
comment:4 by , 9 years ago
Replying to cehoyos:
But it would not be overkill for 10bit iiuc?
Well, I haven't checked the weighting and shift here and there, but 8bits looked really unlikely to need that many bits. 10bits neither, at the bitrates intended, but it looks like it would be 'physically' possible.
That format looks like it was intended to be generic, so it's not unrealistic they made things use common denominators. Or the opposite: flags (like what now looks like interlaced mb) only existing in some profiles etc.
+ if (ctx->bit_depth == 8) {
+ skip_bits1(&ctx->gb);
That bit really seems like 'interlaced mb', so it is either always here, or not dependent on bitdepth alone, but rather resolution/profile.
After some testing, we will only find out if you push;-)
Well, it was useful as jeremy noticed the non-perfect decoding on following frames. See updated patch (will be renamed).
by , 9 years ago
Attachment: | 0001-dnxhddec-skip-an-unknown-bit.patch added |
---|
Now with 100% more interlaced mbs
comment:5 by , 9 years ago
On the right edge between his cheek and the background, above his raised finger, there's a block with strange 'blockiness'. I suspect weighting and scan order might cause that issue, but the interlaced case is missing from the tables. Someone may have to fill them in.
Or that artifact is already in the source/expected output, but I doubt it.
comment:6 by , 9 years ago
This is on the first frame, right? I'll see if I can grab a full frame still from Avid to compare with.
comment:7 by , 9 years ago
Yes, it's somewhat more visible in later frames (but first frame is ok).
Of course, the current situation is clearly better, but that's not enough. Guessing can no longer help.
People have filled tables, they probably have access to specs, so the meaning of the header bits and what these specs say about interlaced is needed.
comment:8 by , 9 years ago
Full quality out of Media Composer (12MB TIFF) - I'll attach these as lower quality graphics shortly, but you're right that it shouldn't have that blockiness.
http://jeremy.publication.org.uk/DNxHD100_cid1260_first_frame.tif
http://jeremy.publication.org.uk/DNxHD100_cid1260_last_frame.tif
by , 9 years ago
Attachment: | DNxHD100_cid1260_original_first_frame.png added |
---|
Still from an alternative decoder
by , 9 years ago
Attachment: | DNxHD100_cid1260_original_last_frame.png added |
---|
comment:9 by , 9 years ago
Ok, the newest tables do not look like they were zigzag-scanned before being added.
comment:10 by , 9 years ago
Cc: | added |
---|
by , 9 years ago
Attachment: | 0001-Use-correct-zigzag-for-weight-tables-for-new-CIDs-12.patch added |
---|
Correct zigzags in the quantization tables
comment:11 by , 9 years ago
I believe the above patch should have the correct zigzags for the DNx100 compression IDs (1258, 1259 and 1260), and I've merged any that were already listed, however I've not tested this on anything other than the existing 1260 content.
I noticed that the chroma table for 1256 (one of the high bitrate DNx444 formats added last year) looked like it hadn't been zigzagged either, but the luma had - I've updated it, but don't have suitable content to test it. Does anybody else?
comment:12 by , 9 years ago
So I had noticed the same, but assumed only Vittorio's additions were affected.
It's nice you noticed e95018b694c0774477abec5bbf86ecc7946a9a28 was also affected. Strange for Kostya, but seems so. As for 1256's luma, it reuses 1235's. Original VLC ticket at https://trac.videolan.org/vlc/ticket/9620 should allow a bit of testing.
I'll send a patchset with all changes if everybody confirms it improves things.
comment:13 by , 9 years ago
I've taken some colour bars in Media Composer, exported it as DNx444 (CID 1256) and did a pixel grab of one of the colour boundaries, vs what Media Composer exports directly as a TIFF, (with some other colour space correction so looks a bit duller). Certainly the fixed-zigzag version looks softer on the boundary, more like I suspect it should.
by , 9 years ago
Attachment: | dnx444_colour.frame.mov added |
---|
Single frame of colour bars in 1080p DNx444 (CID 1256)
comment:14 by , 9 years ago
Analyzed by developer: | set |
---|---|
Resolution: | → fixed |
Status: | open → closed |
So we went over this a bit more with Jeremy, and think it has been fixed by 2801a1352dc8682b028e53880f9847fcb2116947 and 428424fe75206753ab2039e624031c9643623ea0.
If you notice remaining issues, please open a new ticket. CID 1256 is likely to have issues.
Screenshot of decode issues