[FFmpeg-devel] Typo in h264_parser.c ?

[John Pilkington]

On 27/04/18 00:47, Mark Thompson wrote:
> On 27/04/18 00:33, John Pilkington wrote:
>> Hi: I haven't previously posted here.
>>
>> I've been on the mythtv lists since 2005 and wrote a cutting script that I use daily on mpeg2video SD recordings from DVB-T; it cuts with Project-X, at keyframes.  I have been trying to write a similar script using ffmpeg to cut h264-encoded HD recordings from DVB-T2 transmissions in the UK. Internal cuts have not been good.
>>
>> I'm using mythffmpeg version 3.4.1, as incorporated in current mythtv-master, in Fedora 26.  Results have been similar with ffmpeg 3.3.7 for x86_64 f26 from rpmfusion.
>>
>> I use a first pass to 'ignore_unknown' streams, and a second to do the cutting. During the second pass I have almost always had error reports of 'illegal reordering of pic_nums_idc' or 'reference count overflow.'
>>
>> I reported that and the following details yesterday on the mythtv-devel list, and a re-post here has been suggested.
>>
>> The error reports apparently come from libavcodec/h264_parser.c, lines 186 and 194.  The numbers in the 'if/else if' combination at lines 182/184 look very strange to me:
>>
>>      unsigned int reordering_of_pic_nums_idc = get_ue_golomb_31(gb);
>>
>>              if (reordering_of_pic_nums_idc < 3)
>>                  get_ue_golomb_long(gb)
>>              else if (reordering_of_pic_nums_idc > 3) {....
>>
>> ..and libavcodec/golomb.h has this at line 82:
>>
>> /**
>> * read unsigned exp golomb code, constraint to a max of 31.
>> * the return value is undefined if the stored value exceeds 31.
>> */
>> static inline int get_ue_golomb_31(GetBitContext *gb)
>> {....
>>
>> ***I think that the '3' in the 'else if' line in the parser should probably be 31. ***
> 
> No.
> 
> This is parsing a reference picture list modification block - see H.264 <https://www.itu.int/rec/T-REC-H.264-201704-I/en>, section 7.3.3.1.
> 
> Valid meaningful values are 0-2, and 3 ends the list.  Any other value is invalid.
> 
>> I have patched my copy accordingly.  I have seen no bad effects, most of the error messages have gone, and the internal cuts are much neater.
>>
>> It would be great if someone would look to see if this and/or other patches are appropriate.  IIUC mythtv won't patch the ffmpeg code that it uses.
>>
>> I have more test results available if required.
> 
> Can you share a short sample showing your problem?
> 
> - Mark

Thanks for the speedy reply. I think I've seen this sort of thing in all 
the tests I have done, and there's a link to a sample clip here, with 
other perhaps relevant stuff:  No streams have been removed.

  https://code.mythtv.org/trac/ticket/12010#comment:18

But here's a specific example of results before and after my patch, both 
working on a stripped-to-two-streams file.  Both obviously still have 
problems.   The options in the command line are my current best effort. 
Results are fine except for transients after most cuts.

John

Recording from itv HD DVB-T2 - The Durrells

-----------------------------
ffmpeg 3.3.7 for x86_64 f26 from rpmfusion.  Not patched.


ionice -c3 ffmpeg -hide_banner -ignore_unknown    -ss 1952.3000 
-noaccurate_seek -t 687.6800  -i 
/home/john/SGs/RecsSG2/21605_20180422185800.ts     -ignore_unknown -ss 
1952.3000 -t 687.6800  -i /home/john/SGs/RecsSG2/21605_20180422185800.ts 
     -map 0:0 -c:v  copy    -map 1:1 -c:a  copy    -avoid_negative_ts 
make_zero   /mnt/sdb1/TScut_work/21605_20180422185800_25222_004.ts

[h264 @ 0x56030df29d60] mmco: unref short failure
     Last message repeated 1 times
[h264 @ 0x56030df29d60] Increasing reorder buffer to 3
[NULL @ 0x56030df29d60] illegal reordering_of_pic_nums_idc 4
[NULL @ 0x56030df29d60] illegal reordering_of_pic_nums_idc 7 
 

[NULL @ 0x56030df29d60] illegal reordering_of_pic_nums_idc 15 
 

[NULL @ 0x56030df29d60] reference count 2 overflow 
 

[NULL @ 0x56030df29d60] reference count 1 overflow 
 

[NULL @ 0x56030df29d60] illegal reordering_of_pic_nums_idc 6 
 

[NULL @ 0x56030df29d60] reference count 1 overflow 
 

[NULL @ 0x56030df29d60] illegal reordering_of_pic_nums_idc 6 
 

[NULL @ 0x56030df29d60] illegal reordering_of_pic_nums_idc 19 
 

Input #0, mpegts, from '/home/john/SGs/RecsSG2/21605_20180422185800.ts':
   Duration: 01:05:59.34, start: 1.400000, bitrate: 3945 kb/s
   Program 1
     Metadata:
       service_name    : Service01
       service_provider: FFmpeg
     Stream #0:0[0x100]: Video: h264 (High) ([27][0][0][0] / 0x001B), 
yuv420p(tv, bt709, top first), 1920x1080 [SAR 1:1 DAR 16:9], 25 fps, 25 
tbr, 90k tbn, 50 tbc
     Stream #0:1[0x101](eng): Audio: aac_latm (LC) ([17][0][0][0] / 
0x0011), 48000 Hz, stereo, fltp

-------------------

mythffmpeg version 3.4.1, as incorporated in current mythtv-master, but 
with patch

ionice -c3 mythffmpeg -hide_banner -ignore_unknown    -ss 1952.3000 
-noaccurate_seek -t 687.6800  -i 
/home/john/SGs/RecsSG2/21605_20180422185800.ts     -ignore_unknown -ss 
1952.3000 -t 687.6800  -i /home/john/SGs/RecsSG2/21605_20180422185800.ts 
     -map 0:0 -c:v  copy    -map 1:1 -c:a  copy    -avoid_negative_ts 
make_zero   /mnt/sdb1/TScut_work/21605_20180422185800_26959_004.ts

[h264 @ 0x8f71c0] mmco: unref short failure
     Last message repeated 1 times
[h264 @ 0x8f71c0] Increasing reorder buffer to 3
[NULL @ 0x8f71c0] illegal memory management control operation 18
[NULL @ 0x8f71c0] reference count 2 overflow 
 

[NULL @ 0x8f71c0] reference count 1 overflow 
 

     Last message repeated 3 times
[NULL @ 0x8f71c0] illegal reordering_of_pic_nums_idc 32
Input #0, mpegts, from '/home/john/SGs/RecsSG2/21605_20180422185800.ts':
   Duration: 01:05:59.34, start: 1.400000, bitrate: 3945 kb/s
     Stream #0:0[0x100]: Video: h264 (High), yuv420p(tv, bt709, top 
first), 1920x1080 [SAR 1:1 DAR 16:9], 25 fps, 25 tbr, 90k tbn, 50 tbc
     Stream #0:1[0x101](eng): Audio: aac_latm (LC), 48000 Hz, stereo, fltp

-----------------