[vlc-devel] [PATCH v2] codec/videotoolbox: Fix uninitialized pts in late start case

Thomas Guillem thomas at gllm.fr
Mon Jul 29 11:56:09 CEST 2019


So, LGTM.

On Mon, Jul 29, 2019, at 11:48, Thomas Guillem wrote:
> 
> 
> On Mon, Jul 29, 2019, at 11:44, Marvin Scholz wrote:
> > 
> > 
> > On 29 Jul 2019, at 11:40, Thomas Guillem wrote:
> > 
> > > On Mon, Jul 29, 2019, at 11:30, Marvin Scholz wrote:
> > >> On 29 Jul 2019, at 11:25, Thomas Guillem wrote:
> > >>
> > >>> On Mon, Jul 29, 2019, at 11:20, Marvin Scholz wrote:
> > >>>> ---
> > >>>>  modules/codec/videotoolbox.m | 4 ++--
> > >>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> > >>>>
> > >>>> diff --git a/modules/codec/videotoolbox.m 
> > >>>> b/modules/codec/videotoolbox.m
> > >>>> index b01b5d4511..111c58fee1 100644
> > >>>> --- a/modules/codec/videotoolbox.m
> > >>>> +++ b/modules/codec/videotoolbox.m
> > >>>> @@ -1269,8 +1269,6 @@ static int StartVideoToolbox(decoder_t 
> > >>>> *p_dec)
> > >>>>      if (HandleVTStatus(p_dec, status, NULL) != VLC_SUCCESS)
> > >>>>          return VLC_EGENERIC;
> > >>>>
> > >>>> -    PtsInit(p_dec);
> > >>>> -
> > >>>>      return VLC_SUCCESS;
> > >>>>  }
> > >>>>
> > >>>> @@ -1448,6 +1446,8 @@ static int OpenDecoder(vlc_object_t *p_this)
> > >>>>                          (char *)&p_dec->fmt_in.i_codec);
> > >>>>      else
> > >>>>          CloseDecoder(p_this);
> > >>>> +
> > >>>> +    PtsInit(p_dec);
> > >>>
> > >>> Don't do PtsInit() in the CloseDecoder() path
> > >>
> > >> Ok, sent a new version.
> > >>
> > >>>
> > >>> Could you squash this commit and the first one into one commit ?
> > >>
> > >> It is meant to replace the previous version, not to be applied both.
> > >> It should apply fine on git master without the previous patch, unless
> > >> I messed something up locally…
> > >
> > > I prefer only one patch. Here is the reason: Your original patch will 
> > > cause the pts to be init two times in the normal path, then you fix 
> > > this regression with the 2nd patch.
> > 
> > Sorry I don't understand what you mean. This is one patch/commit. All I 
> > did was send
> > a new version with the requested changes, replacing the previous patch.
> 
> Ah yes sorry.
> I was puzzled because the orignal "+ if (!p_sys->session /* Late Start 
> */) { PtsInit(p_dec); }" was missing but you don't need it anymore 
> since the pts is init from the Open callback for all cases now.
> 
> > 
> > >
> > >>
> > >>>
> > >>>>      return i_ret;
> > >>>>  }
> > >>>>
> > >>>> -- 
> > >>>> 2.19.1
> > >>>>
> > >>>> _______________________________________________
> > >>>> vlc-devel mailing list
> > >>>> To unsubscribe or modify your subscription options:
> > >>>> https://mailman.videolan.org/listinfo/vlc-devel
> > >>> _______________________________________________
> > >>> vlc-devel mailing list
> > >>> To unsubscribe or modify your subscription options:
> > >>> https://mailman.videolan.org/listinfo/vlc-devel
> > >> _______________________________________________
> > >> vlc-devel mailing list
> > >> To unsubscribe or modify your subscription options:
> > >> https://mailman.videolan.org/listinfo/vlc-devel
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list