[vlc-devel] Re: Skipped/Drop Frames

[Dermot McGahon]

> Sorry, Dermont I did not want to be rude. I had asked that question as
> I had improved on my last patch and was wondering if there was a
> better way to send on the patch, other then emailing you and
> cluttering up your mailbox unnecessarily.

You're not rude at all. Ideally I would have gotten time to look at
this by now but I've been busy.

You can send patches directly to the the vlc list. But! What you have
been sending so far have not been patches :)

Have a look at:

   $ man diff
   $ man patch

Basically, you take a clear version of livedotcom.cpp and your edited
version, called for example, livedotcom_edit.cpp, and:

  $ diff -u livedotcom.cpp livedotcom_edit.cpp > livedotcom.patch

You might need to reverse those two file names. You will know from the
patch generated, have a look at it. It will be obvious from the patch
if the filenames are the right way round.

This will give you a patch. Submit it to the mailing list.

Clean up things like the //hussian comments, before creating the patch.

Just so you know, you apply a patch by putting it in the directory with
the file to be patched and use:

   $ patch -p1 < livedotcom.patch

to apply it.

This is for single file patches. For a whole directory tree, you
use:

   $ diff -ur . > your_patch.patch



> This patch is I believe slightly more elegant than the first one, and
> requires minimal change to the livedotcomm.cpp file. However it is not
> too robust. In some cases after the pause, if the picture starts
> dropping or so, things slightly go out of sync.

Does it regain sync soon afterwards?

I'm not sure about this. We might need some more testing and log info
to diagnose what is happening.

Can you provide vlc -vvv output from just before the PAUSE takes affect
and just after the subsequent PLAY occurs?



> All you have to do is change/replace the DEMUX_SET_PAUSE_STATE case in
> the Control, in livedotcom.cpp with the code below:
>
> case DEMUX_SET_PAUSE_STATE
>             double d_npt; /* hussain */
>             MediaSubsessionIterator *iter;
>             MediaSubsession *sub;
>
>             b_bool = (vlc_bool_t)va_arg( args, vlc_bool_t );
>             if( p_sys->rtsp == NULL )
>                 return VLC_EGENERIC;
>
> 	    d_npt =( (double)( p_sys->i_pcr - p_sys->i_pcr_start +
> p_sys->i_start) ) / 1000000.00; /* hussain */
>
>             iter = new MediaSubsessionIterator( *p_sys->ms );
>             while( ( sub = iter->next() ) != NULL )
>             {
>                 if( ( b_bool && !p_sys->rtsp->pauseMediaSubsession( *sub  
> ) ) ||
>                     ( !b_bool && !p_sys->rtsp->playMediaSubsession(
> *sub, d_npt ) ) )
>                 {
>                     delete iter;
>                     return VLC_EGENERIC;
>                 }
>             }
>             delete iter;
>
>             /* reset PCR and PCR start, mmh won't work well for
> multi-stream I fear */
>             for( i = 0; i < p_sys->i_track; i++ )
>             {
>                 live_track_t *tk = p_sys->track[i];
>                 tk->i_pts = 0;
>             }
>
> 	    p_sys->i_pcr_start =p_sys->i_pcr_start ; // hussain ... this fix
> might need some more tunning (for the slider)
> 	    p_sys->i_pcr =p_sys->i_pcr; // hussain this ensures that the
> streams starts from the place it was paused at

This does nothing? Assigns A = A and B = B?



> 	    //p_sys->i_pcr_start = 0; /* FIXME Wrong */
> 	    //p_sys->i_pcr = 0;

Remove these lines, and tidy up your comments.



>              return VLC_SUCCESS;
>
> Any comments on this code, (if possible), would be appreciated as it
> might help me improve on it and make it a bit more robust.  And
> especially as I would also like to know if this patch was any good or
> not. So that I can improve on it, and not waste time my sending you
> worthless patches.

The changes are worthwhile. It's good that you're doing this. Just make
a proper patch, post it to the list and others will review it. What you
have included above isn't everything, is it? Includes? Were i_pcr,
i_pcr_start, i_start already given values before your patch? No
changes were required to any other files?


> Sorry for bothering you like this, and thanks for you time and patience.

You're not bothering me at all. I'm glad to see somebody working on
this functionality.


Dermot.
--

-- 
This is the vlc-devel mailing-list, see http://www.videolan.org/vlc/
To unsubscribe, please read http://developers.videolan.org/lists.html