[vlc-devel] VideoToolbox green corruption bug

[Thomas Guillem]

On Fri, Jul 7, 2017, at 22:00, Oliver Collyer wrote:
> Hi
> 
> I'm trying to fix this bug in VideoToolbox:
> 
> https://trac.videolan.org/vlc/ticket/18416
> 
> It was introduced by this commit:
> 
> 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?
Yes it is, we have a create a new avcC containing all SPS/PPS, see avcCFromAnnexBCreate() function.This new avcC should contain all previous SPS/PPS. So, normally, decoder should be restarted only if there is a new one. Maybe there is a bug in hxxx_helper that fail to see if a SPS/PPS already exists.As your stream can be played without restart, I would assume that there is an issue in hxxx_helper
> 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);
>         }
> 
> _________________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170710/20946a55/attachment-0001.html>