Multi-thread decoding artifact for MTS H.264

[Andreas Girgensohn]

When extracting images from an interlaced H.264 MTS file, there are artifacts in some of the frames. The images are extracted inside my own application. I could not figure out how to make ffmpeg run multi-threaded when the output is a sequence of images.

The test video is the one linked from ticket #993: ​http://file.meyersproduction.com/hg10/00009.MTS.zip

ffprobe version 0.10.3 Copyright (c) 2007-2012 the FFmpeg developers
  built on May 10 2012 10:41:04 with gcc 4.6.3 20120306 (Red Hat 4.6.3-2)
  configuration: 
  libavutil      51. 35.100 / 51. 35.100
  libavcodec     53. 61.100 / 53. 61.100
  libavformat    53. 32.100 / 53. 32.100
  libavdevice    53.  4.100 / 53.  4.100
  libavfilter     2. 61.100 /  2. 61.100
  libswscale      2.  1.100 /  2.  1.100
  libswresample   0.  6.100 /  0.  6.100
[mpegts @ 0x32174e0] Format mpegts probed with size=2048 and score=100
[mpegts @ 0x32174e0] stream=0 stream_type=1b pid=1011 prog_reg_desc=HDMV
[mpegts @ 0x32174e0] stream=1 stream_type=81 pid=1100 prog_reg_desc=HDMV
[h264 @ 0x321b3a0] err{or,}_recognition separate: 1; 1
[h264 @ 0x321b3a0] err{or,}_recognition combined: 1; 10001
[ac3 @ 0x321bd60] err{or,}_recognition separate: 1; 1
[ac3 @ 0x321bd60] err{or,}_recognition combined: 1; 10001
[ac3 @ 0x321bd60] Unsupported bit depth: 0
[h264 @ 0x321b3a0] Increasing reorder buffer to 1
[h264 @ 0x321b3a0] no picture ooo
    Last message repeated 1 times
[h264 @ 0x321b3a0] no picture 
[mpegts @ 0x32174e0] max_analyze_duration 5000000 reached at 5003333
rfps: 59.916667 0.001150
    Last message repeated 1 times
rfps: 59.940060 0.000000
    Last message repeated 1 times
Input #0, mpegts, from '00009.MTS':
  Duration: 00:00:36.54, start: 0.766967, bitrate: 6884 kb/s
  Program 1 
    Stream #0:0[0x1011], 302, 1/90000: Video: h264 (High) (HDMV / 0x564D4448), yuv420p, 1440x1080 [SAR 4:3 DAR 16:9], 1001/60000, 59.96 fps, 59.94 tbr, 90k tbn, 59.94 tbc
    Stream #0:1[0x1100], 158, 1/90000: Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, stereo, s16, 256 kb/s
[h264 @ 0x321b3a0] err{or,}_recognition separate: 1; 10001
[h264 @ 0x321b3a0] err{or,}_recognition combined: 1; 10001
[h264 @ 0x321b3a0] detected 8 logical cores
[ac3 @ 0x321bd60] err{or,}_recognition separate: 1; 10001
[ac3 @ 0x321bd60] err{or,}_recognition combined: 1; 10001
[ac3 @ 0x321bd60] Unsupported bit depth: 0

This works without problems with FFmpeg 0.9 but both versions 0.10 and 0.10.3 have the problem when using eight threads instead of one.

The attached files show the same extracted frame with FFmpeg 0.9 (multiple threads), 0.10.3 (multiple threads), and 0.10.3 (single thread). In all cases, the image is scaled to 1440x810 to correct the aspect ratio.

Please let me know if you need additional information.

[Andreas Girgensohn]

0.9 8 threads

[Andreas Girgensohn]

0.10.3 1 thread

[Andreas Girgensohn]

0.10.3 8 threads

[Carl Eugen Hoyos]

Please use git bisect to find the version introducing the problem.

[Andreas Girgensohn]

Unfortunately, I could not determine the exact revision where the problem was introduced due to a compile error in my application for several revisions.

ffmpeg-video-frame-extractor.cpp:232:66: error: ‘av_rescale’ was not declared in this scope
ffmpeg-video-frame-extractor.cpp:288:39: error: ‘AVFrame’ has no member named ‘best_effort_timestamp’

This is the final output of "git bisect":

There are only 'skip'ped commits left to test.
The first bad commit could be any of:
7e813d59335753ab07be82919f36ddcaf075463a
4340a6363e9ff75dc4e8ce14dc96671623494ed8
5b4c5628973d212ecaa1aabaeebd0205738ceddd
220506d23f39da3e23d3d42fb7061f19cec8052c
9ae846971fc1dd1dff5cac9b8f26cff499e053c5
02e7dbf5adc6aa702472010c33aec9bfd904702f
53ed79a260099c761f8a74872d695a2aeb7a0ced
d481227c549aece0bc4a05819a771806b6cd7507
e1ac69fa27b924999bc5dbb7ffefa2af88bf3798
1381e9bc92e54f0c3472a1f7150a8530ccd55379
98c290cc0828a25b04259f2b408054aaeca016e4
0e6a8b5cf76f830923c85730e6369ca46cfe834c
dc7ad30fa5b3d454a3edcc3b051aef3a65c5235b
10fef6bd6b3913c94d81276a271ac0c77c2c3525
af0292f33a7d9a024a10a05748415622ea45e08e
75e2025f57fef5060d74c65d9654e61c35bd6c81
be14a37066cc99e2b769ee5e044a34caecd24938
941e9f22386e923183dc4c5392d0ef2a85b68d51
b8dd555c63ca4ef1034006159b01f49e103c6252
cdfe94c5ab1df40c6c724df5d4cafe2539c5571a
d1ad6bdb6b578919f706694656a071d7ff7d9d84
1d3a9e63e0dcbcba633d939cdfb79e977259be13
efe68076dab56293168ffb66d7b6c1977b740098
1d9569f9e8361c3be06b9732c0b80639a51b4b87
We cannot bisect more!

Here is the log of the revisions that I tested.

git bisect start
# bad: [5ce1b214eb8b297fc860172d177523b306af842c] RELEASE_NOTES: update for 0.10
git bisect bad 5ce1b214eb8b297fc860172d177523b306af842c
# good: [39f59a8da7f024532b0d62ad429a7a8ffaa2d631] RELEASE: We're now at 0.9.0.git
git bisect good 39f59a8da7f024532b0d62ad429a7a8ffaa2d631
# good: [3edff185abfdd089b88ecc5770e5f6a963055a97] Merge remote-tracking branch 'qatar/master'
git bisect good 3edff185abfdd089b88ecc5770e5f6a963055a97
# good: [a8b117f800152bd19f1f99f9a76b5b1933927441] Add gray16 and rgb48 png encoding regression tests.
git bisect good a8b117f800152bd19f1f99f9a76b5b1933927441
# good: [eaf4bf6df2cf26c31bf7f787edd69812a681ab2e] CrystalHD: Initialise variables to silence valgrind.
git bisect good eaf4bf6df2cf26c31bf7f787edd69812a681ab2e
# good: [76c3e76eb35ce7cca5c912f0d21b736bb0be22fb] Allow user to force reading mov alias from absolute path.
git bisect good 76c3e76eb35ce7cca5c912f0d21b736bb0be22fb
# skip: [d55fa1cb25244eab9f919d6e04f1e9b3faf7b7c9] tools: Remove some unnecessary #undefs.
git bisect skip d55fa1cb25244eab9f919d6e04f1e9b3faf7b7c9
# bad: [807a045ab7f51993a2c1b3116016cbbd4f3d20d6] kgv1dec: Increase offsets array size so it is large enough.
git bisect bad 807a045ab7f51993a2c1b3116016cbbd4f3d20d6
# skip: [b8dd555c63ca4ef1034006159b01f49e103c6252] aud: remove unneeded field, audio_stream_index from context
git bisect skip b8dd555c63ca4ef1034006159b01f49e103c6252
# skip: [7e813d59335753ab07be82919f36ddcaf075463a] vqa: clean up audio header parsing
git bisect skip 7e813d59335753ab07be82919f36ddcaf075463a
# skip: [be14a37066cc99e2b769ee5e044a34caecd24938] aud: fix time stamp calculation for ADPCM IMA WS
git bisect skip be14a37066cc99e2b769ee5e044a34caecd24938
# bad: [59e95fa4a8844d2abe7ddd7b8d269ea8d8eea17d] h263dec: Disallow width/height changing with frame threads.
git bisect bad 59e95fa4a8844d2abe7ddd7b8d269ea8d8eea17d
# skip: [02e7dbf5adc6aa702472010c33aec9bfd904702f] adpcm_ima_ws: fix stereo decoding
git bisect skip 02e7dbf5adc6aa702472010c33aec9bfd904702f
# skip: [941e9f22386e923183dc4c5392d0ef2a85b68d51] lavd: remove deprecated v4l grab device.
git bisect skip 941e9f22386e923183dc4c5392d0ef2a85b68d51
# skip: [0e6a8b5cf76f830923c85730e6369ca46cfe834c] aud: set pts_wrap_bits to 64.
git bisect skip 0e6a8b5cf76f830923c85730e6369ca46cfe834c
# skip: [75e2025f57fef5060d74c65d9654e61c35bd6c81] avplay: remove the -er option.
git bisect skip 75e2025f57fef5060d74c65d9654e61c35bd6c81
# skip: [98c290cc0828a25b04259f2b408054aaeca016e4] cosmetics: indentation
git bisect skip 98c290cc0828a25b04259f2b408054aaeca016e4
# bad: [2179b638e3a495412d616272641742db42963aaf] v4l: fix compilation
git bisect bad 2179b638e3a495412d616272641742db42963aaf
# skip: [4340a6363e9ff75dc4e8ce14dc96671623494ed8] vqa: remove unused context fields, audio_samplerate and audio_bits
git bisect skip 4340a6363e9ff75dc4e8ce14dc96671623494ed8
# skip: [5b4c5628973d212ecaa1aabaeebd0205738ceddd] vqa: set time base to frame rate as coded in the header.
git bisect skip 5b4c5628973d212ecaa1aabaeebd0205738ceddd
# skip: [af0292f33a7d9a024a10a05748415622ea45e08e] lavc: postpone removing old audio encoding and decoding API
git bisect skip af0292f33a7d9a024a10a05748415622ea45e08e
# skip: [220506d23f39da3e23d3d42fb7061f19cec8052c] avcodec: add a new codec_id for CRYO APC IMA ADPCM.
git bisect skip 220506d23f39da3e23d3d42fb7061f19cec8052c
# skip: [dc7ad30fa5b3d454a3edcc3b051aef3a65c5235b] lavf: postpone removing av_close_input_file().
git bisect skip dc7ad30fa5b3d454a3edcc3b051aef3a65c5235b
# skip: [9ae846971fc1dd1dff5cac9b8f26cff499e053c5] vqa: set packet duration.
git bisect skip 9ae846971fc1dd1dff5cac9b8f26cff499e053c5
# skip: [cdfe94c5ab1df40c6c724df5d4cafe2539c5571a] aacenc: Write correct length for long identification strings.
git bisect skip cdfe94c5ab1df40c6c724df5d4cafe2539c5571a
# skip: [53ed79a260099c761f8a74872d695a2aeb7a0ced] vqa: use 1/sample_rate as the audio stream time base
git bisect skip 53ed79a260099c761f8a74872d695a2aeb7a0ced
# skip: [10fef6bd6b3913c94d81276a271ac0c77c2c3525] aud: simplify header parsing
git bisect skip 10fef6bd6b3913c94d81276a271ac0c77c2c3525
# skip: [d481227c549aece0bc4a05819a771806b6cd7507] aud: support Westwood SND1 audio in AUD files.
git bisect skip d481227c549aece0bc4a05819a771806b6cd7507
# skip: [efe68076dab56293168ffb66d7b6c1977b740098] aacenc: Fix identification padding when the bitstream is already aligned.
git bisect skip efe68076dab56293168ffb66d7b6c1977b740098
# skip: [e1ac69fa27b924999bc5dbb7ffefa2af88bf3798] vqa: set stream start_time to 0.
git bisect skip e1ac69fa27b924999bc5dbb7ffefa2af88bf3798
# skip: [d1ad6bdb6b578919f706694656a071d7ff7d9d84] Changelog: restore version <next> header
git bisect skip d1ad6bdb6b578919f706694656a071d7ff7d9d84
# skip: [1381e9bc92e54f0c3472a1f7150a8530ccd55379] lavc: postpone the removal of AVCodecContext.request_channels.
git bisect skip 1381e9bc92e54f0c3472a1f7150a8530ccd55379
# skip: [1d3a9e63e0dcbcba633d939cdfb79e977259be13] rv10: verify slice offsets against buffer size
git bisect skip 1d3a9e63e0dcbcba633d939cdfb79e977259be13
# bad: [1d9569f9e8361c3be06b9732c0b80639a51b4b87] Merge remote-tracking branch 'qatar/master'
git bisect bad 1d9569f9e8361c3be06b9732c0b80639a51b4b87

[Andreas Girgensohn]

The results listed above did not seem to be completely plausible. Thus, I fixed my application to access "best_effort_timestamp" via "av_opt_ptr" and to use "pkt_pts" in the one revision that didn't provide that. I also stopped using "av_rescale".

Here are the new results. Unfortunately, this is still not a commit that mentions h264.

969ba65ecc8910b7f386d2a12f7012f702bccab3 is the first bad commit
commit 969ba65ecc8910b7f386d2a12f7012f702bccab3
Author: Nicolas George <nicolas.george@normalesup.org>
Date:   Tue Jan 24 13:21:34 2012 +0100

    libcelt: configure: distinguish not found and too old.
    
    Fixes ticket #940.

:100755 100755 a06e63cbe067c7f5f6a2e9ac7536371912783371 2a3975edcede653c54207a5ab13cbaf4ff9a8934 M configure

git bisect start
# bad: [5ce1b214eb8b297fc860172d177523b306af842c] RELEASE_NOTES: update for 0.10
git bisect bad 5ce1b214eb8b297fc860172d177523b306af842c
# good: [39f59a8da7f024532b0d62ad429a7a8ffaa2d631] RELEASE: We're now at 0.9.0.git
git bisect good 39f59a8da7f024532b0d62ad429a7a8ffaa2d631
# good: [3edff185abfdd089b88ecc5770e5f6a963055a97] Merge remote-tracking branch 'qatar/master'
git bisect good 3edff185abfdd089b88ecc5770e5f6a963055a97
# good: [a8b117f800152bd19f1f99f9a76b5b1933927441] Add gray16 and rgb48 png encoding regression tests.
git bisect good a8b117f800152bd19f1f99f9a76b5b1933927441
# good: [eaf4bf6df2cf26c31bf7f787edd69812a681ab2e] CrystalHD: Initialise variables to silence valgrind.
git bisect good eaf4bf6df2cf26c31bf7f787edd69812a681ab2e
# bad: [76c3e76eb35ce7cca5c912f0d21b736bb0be22fb] Allow user to force reading mov alias from absolute path.
git bisect bad 76c3e76eb35ce7cca5c912f0d21b736bb0be22fb
# good: [0fec2cb15cc6ff1fcc724c774ec36abadcb7b6ad] Remove ffmpeg.
git bisect good 0fec2cb15cc6ff1fcc724c774ec36abadcb7b6ad
# good: [0c3577bfd98a8a3791adb7c01b4a203dddb345f8] lavfi: add avfilter_graph_dump.
git bisect good 0c3577bfd98a8a3791adb7c01b4a203dddb345f8
# bad: [cfa2963b7e55cd7b0061578d67b724ef99554dcb] maintainers: add myself for recent works.
git bisect bad cfa2963b7e55cd7b0061578d67b724ef99554dcb
# bad: [b2be1dabb194efe911c5c3015ba322e32701f009] mpegvideo: Draw edges based on the pictures linesize instead of the contexts.
git bisect bad b2be1dabb194efe911c5c3015ba322e32701f009
# good: [af21823ae0dc8f72446f9beb22563a72f485a57c] lavfi: require libswr for af_pan.
git bisect good af21823ae0dc8f72446f9beb22563a72f485a57c
# bad: [20aed9ed4f4cae6a2feabbea5fb48bad75359538] ffmpeg: Allocate buffers of the size needed by the decoder.
git bisect bad 20aed9ed4f4cae6a2feabbea5fb48bad75359538
# bad: [969ba65ecc8910b7f386d2a12f7012f702bccab3] libcelt: configure: distinguish not found and too old.
git bisect bad 969ba65ecc8910b7f386d2a12f7012f702bccab3

[Andreas Girgensohn]

git bisect does not seem to handle the ffmpeg git structure very well. I checked by hand several revisions around the revision indicated by git bisect and I believe that I found the revision that introduced the problem. I have no idea how support for subtitles could produce this problem.

commit d150a147dac67faeaf6b1f25a523ae330168ee1e
Author: David Mitchell <dave@fallingcanbedeadly.com>
Date:   Thu Jan 19 07:31:01 2012 -0800

    Improve support for PGS subtitles.
    
    The previous implementation assumed that a new picture would always
    supersede the previous picture. Similarly, presentation segments
    were assumed to pertain to the most-recently-read picture.
    
    However, each presentation segment may refer to 0 or more pictures
    by their ID. Picture IDs may repeat, and a repeated picture ID
    indicates that the old picture for that ID is no longer needed
    and may be discarded.
    
    The new implementation allocates a buffer with one slot for each
    possible picture ID (the picture ID is a 16-bit field) and
    properly decodes presentation segments so that all relevant
    pictures are output upon encountering a display segment.
    
    Given that most PGS streams are unlikely to use more than a small
    fraction of the available picture IDs, it would probably be better
    to use a more memory-efficient data structure. I'm lazy though, so
    I leave this to a more motivated individual.
    
    I've tested the code with MKV files in VLC (a recent revision from
    their git repo) and with HandBrake (a version that I hacked up to
    use ffmpeg's PGS subtitle decoder).
    
    Review-by: Hendrik Leppkes <h.leppkes@gmail.com>
    Signed-off-by: Michael Niedermayer <michaelni@gmx.at>

Here are the revisions that I tested.

b720915be103cc8b062405bf9f7765ce3ad679d1 bad
af21823ae0dc8f72446f9beb22563a72f485a57c bad
0bb57f8bf029427059be21a562527dcfa0e264c9 bad
b955d4072e3e563b230c9ab4d6575577a3dc7314 bad
f58d67000247eb8ac5029c3f16634d5851d4dfa9 bad
0fec2cb15cc6ff1fcc724c774ec36abadcb7b6ad good
2aadff2e44fa27664ccd1b0a63829e61bf82e939 bad
d150a147dac67faeaf6b1f25a523ae330168ee1e bad
cf7c7f13cdad07a174110625f8452c8e3444717b good
575d494de561049f36f9c5492e05c7d83dd78e75 good
feb997577b66367a0c4269100888640699ee890d good
9bf9c314a093db16a009829bfe874bf03ffaecc9 good

[Carl Eugen Hoyos]

Is the problem not reproducible with current git head?

If it works fine with cf7c7f1, but produces artefacts with d150a14, then you have found the responsible change.

If you want to make sure that this is subtitle-related, you could test with --disable-decoder=pgssub.

[Andreas Girgensohn]

The problem is not reproducible with the current git head but it is reproducible with the 0.10.3 download (a little surprising considering that 0.10.3 is only 6 days old). Specifying --disable-decoder=pgssub for 0.10.3 does not make the problem go away. I verified that libavcodec/pgssubdec.c did not get compiled when specifying that option. Thus, it does not seem to be responsible after all.

[Andreas Girgensohn]

Sorry, I was off by one revision. The problem is unrelated to pgssub. The problem appears to have been introduced here:

commit cf7c7f13cdad07a174110625f8452c8e3444717b
Author: Michael Niedermayer <michaelni@gmx.at>
Date:   Mon Jan 23 06:34:30 2012 +0100

    pthreads: Generic progress lubrication support.
    
    Fixes bug118, bug120 and bug125 at least
    
    Signed-off-by: Michael Niedermayer <michaelni@gmx.at>

It looks like it was fixed here:

commit 18a7f7465e7e6b9c3688ffc23230ae7a0639a771
Author: Michael Niedermayer <michaelni@gmx.at>
Date:   Sat Feb 11 20:14:33 2012 +0100

    threads: Perform the generic progress cleanup more carefully.
    
    The cleanup is only done now when
    a picture is returned (assuming that it has to be done when its returned)
    a error is returned (assuming that there will be no further progress on the frame)
    the codec is not h264 (this is still needed due to some deadlocks in realvideo)
    
    This fixes a decoding regression with 00017.MTS
    
    Signed-off-by: Michael Niedermayer <michaelni@gmx.at>

It there any chance that this fix will be included in the next release of the 0.10 branch?

[Carl Eugen Hoyos]

Replying to andreasg:

It looks like it was fixed here:

commit 18a7f7465e7e6b9c3688ffc23230ae7a0639a771
Author: Michael Niedermayer <michaelni@gmx.at>
Date:   Sat Feb 11 20:14:33 2012 +0100

    threads: Perform the generic progress cleanup more carefully.
    
    The cleanup is only done now when
    a picture is returned (assuming that it has to be done when its returned)
    a error is returned (assuming that there will be no further progress on the frame)
    the codec is not h264 (this is still needed due to some deadlocks in realvideo)
    
    This fixes a decoding regression with 00017.MTS
    
    Signed-off-by: Michael Niedermayer <michaelni@gmx.at>

It there any chance that this fix will be included in the next release of the 0.10 branch?

I cherry-picked the commit to the 0.10 release branch, you can test it with git checkout release/0.10. Please report!

[Andreas Girgensohn]

Thanks, the problem is now solved in the 0.10 release branch.