Skip to content
Snippets Groups Projects

input: thumbnailer: remove seek to same position

2 unresolved threads

When tracing the thumbnailing test, we can see that the media was first opened and demuxed, before DEMUX_SET_POSITION/SET_TIME is sent to the demux which will reset back to the same initial position.

Check that actual seek values are provided to avoid the seek when not needed.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
265 265 {
266 266 struct seek_target seek_target = {
267 267 .type = VLC_THUMBNAILER_SEEK_TIME,
268 .time = time,
268 .time = VLC_TICK_0 + time,
  • It seems that it should be done closer to the call using this value: the one calling input_SetTime().

    But then if you dig into what it does, it calls INPUT_CONTROL_SET_TIME. The same way vlc_player_SeekByTime() does. And the documentation of vlc_player_SeekByTime() doesn't mention that the time has to be shifted and it's in fact not shifted either. So if the problem exists here, it exists there and might be fixed in a more generic way. Probably in the single location that handles this internal control in input.c

  • Please register or sign in to reply
  • Steve Lhomme
    Steve Lhomme @robUx4 started a thread on commit d4120eb3
  • 177 177 goto error;
    178 178
    179 179 if (task->seek_target.type == VLC_THUMBNAILER_SEEK_TIME)
    180 input_SetTime(input, task->seek_target.time, task->fast_seek);
    180 {
    181 if (task->seek_target.time != VLC_TICK_0)
    • This assumes the provided times are shifted with VLC_TICK_0, as seen in the next commit. But IMO this is not the proper place to do the shift. So IMO is should be tested against 0.

      Also it seems that the goal is to avoid a costly seeking operation, because INPUT_CONTROL_SET_TIME ends up doing a PCR Reset no matter what. But it doesn't have to be that way. We can call vlc_demux_GetTime() and check that it's not the same value as the current time. If it is then we don't need the PCR reset and the demux_SetTime(). And you have optimized seeking for input_SetTime() callers and vlc_player_SeekByTime() callers.

    • Please register or sign in to reply
  • added MRStatus::Stale label and removed MRStatus::InReview label

  • Please register or sign in to reply
    Loading