[vlc-devel] [PATCH 01/11] mkv: fixed DEMUX_GET_TIME

[Filip Roséen]

I originally wrote the contents in the latter section of this email but then had
a rather long conversation with `InTheWings` (in `freenode/#videolan`). 

I think the below summarizes everything:

    12:53:27  refp$ InTheWings: but just to be clear, what is the relationship between DEMUX_SET_TIME and DEMUX_GET_TIME?
    12:53:35  InTheWings$ SET_TIME, you seek to the point required to decode that time
    12:53:45  refp$ InTheWings: since they differ in what "time" is being referred to
    12:54:12  InTheWings$ yeah +- dts<->pts diff
    12:54:44  refp$ InTheWings: so the implementation of SlaveSeek (and other stuff related to slaves in src/input/input.c) is wrong?
    12:55:02  InTheWings$ don't know
    12:55:09  InTheWings$ and mkv is not contiguous data
    12:55:37  refp$ InTheWings: https://github.com/videolan/vlc/blob/d257781099d7c13d0127f95f9bc0ba136587d71b/src/input/input.c#L2519
    12:55:55  refp$ InTheWings: it uses DEMUX_GET_TIME for the master, and then passes that timestamp to the slaves with DEMUX_SET_TIME
    12:56:56  InTheWings$ and that's correct. syncs both at pcr level
    12:57:27  InTheWings$ or pcr < time that allows playback of time
    12:57:31  InTheWings$ that's the point

I am still a little bit confused, and as such the two patches in this batch
(that changes `DEMUX_GET_{POSITION,TIME}`) should probably be put on hold (ie.
ignored at the current time).

What follows is just my thoughts prior to the discussion, and I will leave it in
for future reference (if someone else also stumbles into the confusion I am
currently having).

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

> > When someone asks where we are we should reply with the PTS, not the
> > PCR (especially since the PCR is VLC_TS_INVALID after we seek, which is
> > when the core will ask for our position if we have slaves).
> 
> DEMUX_GET_TIME means demuxer time, not ES time.

Hmm, though given how `DEMUX_GET_TIME` is treated in the core, isn't the PTS the
appropriate value to yield? I honestly thought there was a well-defined symmetry
between `DEMUX_GET_TIME` and `DEMUX_SET_TIME`.

My assumptions was based on the code in `src/input/input.c` as well as other
demux implementations that is using the PTS as the value for `DEMUX_GET_TIME`.

The implementation of `SlaveSeek` from `src/input/input.c`:

    2519  static void SlaveSeek( input_thread_t *p_input )
    2520  {
    2521      int64_t i_time;
    2522      int i;
    2523  
    2524      if( demux_Control( p_input->p->master->p_demux, DEMUX_GET_TIME, &i_time ) )
    2525      {
    2526          msg_Err( p_input, "demux doesn't like DEMUX_GET_TIME" );
    2527          return;
    2528      }
    2529  
    2530      for( i = 0; i < p_input->p->i_slave; i++ )
    2531      {
    2532          input_source_t *in = p_input->p->slave[i];
    2533  
    2534          if( demux_Control( in->p_demux, DEMUX_SET_TIME, i_time, true ) )
    2535          {
    2536              if( !in->b_eof )
    2537                  msg_Err( p_input, "seek failed for slave %d -> EOF", i );
    2538              in->b_eof = true;
    2539          }
    2540          else
    2541          {
    2542              in->b_eof = false;
    2543          }
    2544      }
    2545  }

Questions
---------

 - Since the time yield by `DEMUX_GET_TIME` is the time used to seek within the
   slaves (if any), how is the demuxer PTS not an appropriate value?

Sorry if I have misunderstood or missed something regarding the semantics of
`DEMUX_GET_TIME`; some clarification would be very helpful!

An example of where using the PTS will fail will also be helpful, so that I can
properly wrap my head around this (because currently I am as confused as a duck
ending up at a college class).

/F
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20160521/6893b83e/attachment.html>