[MPlayer-users] Feature Request: Get OSD Time as a Slave Command

[Reimar Döffinger]

On Tue, Nov 15, 2011 at 08:44:12PM -0500, mplayer wrote:
> >>>>> ISSUE: The slave command get_time_pos returns the file
> >>>>> timestamp, but for some
> >>> file
> >>>>> formats the timestamp resets.
> >>>>>
> >>>>> SOLUTION (proposed): However, I noticed that the OSD time
> >>>>> displayed is always good.  So why
> >>> not
> >>>>> add a slave command 'get_osd_time' to expose the value the OSD
> >>>>> is using
> >>> to
> >>>>> display -- it appears to me that OSD just asks the demuxer for
> >>>>> the
> >>> current
> >>>>> time.
> >>>>>
> >>>>> So this is what I think we'd need to add;
> >>>>>
> >>>>> input.h MP_CMD_GET_OSD_TIME,
> >>>>>
> >>>>> input.c { MP_CMD_GET_OSD_TIME, "get_osd_time", 0, { {-1,{0}} }
> >>>>> },
> >>>>>
> >>>>> command.c case MP_CMD_GET_OSD_TIME: mp_msg(MSGT_GLOBAL,
> >>>>> MSGL_INFO, "ANS_OSD_TIME=%d\n",
> >>>>> demuxer_get_current_time(mpctx->demuxer));
> >>>>
> >>>> There is no need to add anything. All that
> >>>> demuxer_get_current_time does is prefer the stream time over
> >>>> pts. You will get the same by just using get_property
> >>>> stream_time_pos and only falling back to get_property time_pos
> >>>> when it is not available.
> >>>
> >>> Something is not adding up.
> >>>
> >>> OSD is using 'demuxer_get_current_time' which implements; if
> >>> (demuxer->stream_pts != MP_NOPTS_VALUE) get_time_ans =
> >>> demuxer->stream_pts; else if (sh_video) get_time_ans =
> >>> sh_video->pts;
> >>>
> >>> Which returns the RIGHT answer.
> >>>
> >>> And the slave command 'time_get_pos' is using; if (sh_video) pos =
> >>> sh_video->pts; ...
> >>>
> >>> Which returns the WRONG answer.
> >>>
> >>> And 'get_property stream_time_pos' is using; if (!mpctx->demuxer ||
> >>> mpctx->demuxer->stream_pts == MP_NOPTS_VALUE) return
> >>> M_PROPERTY_UNAVAILABLE; return m_property_time_ro(prop, action,
> >>> arg, mpctx->demuxer->stream_pts);
> >>>
> >>> But when I try calling the slave command: 'get_property
> >>> stream_time_pos' I get the following; Failed to get value of
> >>> property 'stream_time_pos'. ANS_ERROR=PROPERTY_UNKNOWN
> >>>
> >>> It's pretty frustrating seeing the OSD display 1:15:23 while
> >>> time_get_pos returns 3:15.
> >>>
> >>> Could we change the 'time_get_pos' to use the same logic as the OSD
> >>> and give preference to the stream pts; if (demuxer->stream_pts !=
> >>> MP_NOPTS_VALUE) pos = demuxer->stream_pts; else if (sh_video) pos =
> >>> sh_video->pts; ...
> >>
> >>
> >> I can confirm that get_property stream_time_pos, does not appear to
> >work.
> >>
> >> mplayer ~/Videos/Guilty\ Crown/\[Doki\]\ Guilty\ Crown\ -\ 01\
> >> \(848x480\ h264\ AAC\)\ \[0D556C93\].mkv -slave -quiet
> >
> >When there are no stream time stamps then it of course is not available.
> >Files streams do not come with time stamps, basically only DVDs will
> >give a result.
> 
> When I tested your suggestion to use "get_property stream_time_pos" it was
> on a DVD of the movie "300".  The OSD display is accurate, "get_property
> stream_time_pos" returns an error

I really can't see how that could happen, and it works just fine here.

> and "time_get_pos" thinks its only 3
> minutes into the movie after about 50% of the stream.

Well, those are the time-stamps encoded in the video itself.

> I'd prefer to see "time_get_pos" changed to:
>   if (demuxer->stream_pts != MP_NOPTS_VALUE) 
>     pos = demuxer->stream_pts; 
>   else if (sh_video) 
>     pos = sh_video->pts; 
>   ...

That might work fine for some use case, but not for others.
Just like the OSD this will for example we off quite a bit
when using a large -cache.
It also has the issue that for streams with "lose" the stream
pts it will end up switching between these.
So I am not at all convinced it is a good idea to encourage that.
I even more dislike doing that when first of all it is a workaround
against "get_property stream_time_pos", because the right solution to
broken functionality is to fix it and not add another one that happens
to work.
I am not exactly against a demuxer_time_pos function calling
demuxer_get_current_time, since I had some further plans for that
function at some point (it really should use pts value to improve on
the stream pts which is uneven and has cache issues), but
1) it will be some time before that function actually offers much
2) currently it will always return 0 for audio-only. I wonder if that
is really intentional...