[vlc-devel] VideoToolbox green corruption bug

Oliver Collyer ovcollyer at mac.com
Fri Jul 7 22:00:30 CEST 2017


Hi

I'm trying to fix this bug in VideoToolbox:

https://trac.videolan.org/vlc/ticket/18416 <https://trac.videolan.org/vlc/ticket/18416>

It was introduced by this commit:

https://github.com/videolan/vlc/commit/b6d73612d383b25ab12e2f8aad289d045200281a <https://github.com/videolan/vlc/commit/b6d73612d383b25ab12e2f8aad289d045200281a>

I've traced it through to the code block below in hxxx_helper.c, inside h264_helper_parse_nal:

What is happening is that every few seconds (every keyframe?) it is detecting a new PPS and thus setting *p_config_changed to true, which causes the decoder to be restarted. This leads to the green corruption.

I found that:

- helper_search_pps detects that the PPS is different. i_nal is normally 82 but every few seconds a PPS arrives of size 53 and this one triggers the change
- despite the above, all entries of the h264_picture_parameter_set_t structure are the same, although I noticed that in h264_parse_picture_parameter_set_rbsp in h264_nal.c it only parses and reads in a few of them
- if I disable the line that sets *p_config_changed = true it fixes the bug with no visible side-effects

So a couple of Qs to anyone familiar with this area:

1) is it correct behaviour to restart the decoder whenever the PPS changes?
2) if yes, what are the circumstances where it should restart the decoder/set config_changed to true, because it would seem that this shouldn't be one of them.
3) if no, then it's a simple patch not to set *p_config_changed to true, correct?

Note that this issue seems specific to interlaced content. I've not seen it with progressive content and as you will see from the bug report it has been independently reported with other interlaced h.264 files too.

Regards

Oliver

        else if (i_nal_type == H264_NAL_PPS)
        {
            if (helper_search_pps(hh, p_nal, i_nal) != NULL)
                continue;
            h264_picture_parameter_set_t *p_pps =
                h264_decode_pps(p_nal, i_nal, true);
            if (!p_pps)
                return VLC_EGENERIC;

            struct hxxx_helper_nal *hnal = &hh->h264.pps_list[p_pps->i_id];

            if (helper_dup_buf(hnal, p_nal, i_nal))
            {
                h264_release_pps(p_pps);
                return VLC_EGENERIC;
            }
            if (hnal->h264_pps)
                h264_release_pps(hnal->h264_pps);
            else
                hh->h264.i_pps_count++;

            hnal->h264_pps = p_pps;
            *p_config_changed = true;
            msg_Dbg(hh->p_obj, "new PPS parsed: %u", p_pps->i_id);
        }

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170707/9d87cad6/attachment.html>


More information about the vlc-devel mailing list