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

[Roger Pack]

On 11/16/11, Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> 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...


Ok here's my take at it, feel free to rename it or what not if you so
desire.  I also wasn't exactly sure how to name it so it would match
convention.
Thanks for your help with this.
-roger-
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-add-get-osd-pos-command.patch
Type: application/octet-stream
Size: 3249 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-users/attachments/20121226/b91436e0/attachment.obj>