[mythtv] Trying to debug seek problems

[Isaac Richards]

On Saturday 28 January 2006 22:11, David Abrahams wrote:
> Isaac Richards <ijr at case.edu> writes:
> > On Saturday 28 January 2006 20:28, David Abrahams wrote:
> >> It's pretty common, when I hit the 'E' key while watching a recording,
> >> for the indicator to end up at zero.  Likewise it's common for me to
> >> end up back at the beginning of all video when I use the skip
> >> forward/reverse keys.
> >
> > <snip>
> >
> >> As I said, this bug is apparently repeated through most of the
> >> subclasses of VideoOutput, the exceptions being VideoOutputIvtv and
> >> VideoOutputXv.  I suppose most people are using Xv and that's why it
> >> hasn't been more widely reported.  Regardless, the repetition of this
> >> code pattern strongly suggests that some refactoring is in order.
> >
> > No.  VideoOutputXv has the same "bug",
>
> Well, it depends. When VideoOutputSubType() > XVideo it calls
> PrepareFrameXvMC, where this->framesPlayed is never updated if buffer
> turns out to be NULL.  So, is that the way it's supposed to be?

Yes.  XvMC pauses differently than Xv does.

> > but that's how the code is supposed to
> > be.  VideoOutputIvtv doesn't even have analogous code.
> >
> > The problem is that VideoOutputQuartz::UpdatePauseFrame() isn't updating
> > the scratch frame's frame number like it should, since it's using that
> > style of pausing.
>
> Okay, so how should it update the scratch frame's frame number?  And
> why, if it's supposed to work that way, is it apparently perfect with
> my change?  What is broken that I'm not seeing?

Check how it's done in VideoOutputXv::UpdatePauseFrame.  Obviously, it works 
there.  Simple grepping would have found that.

> > This obviously is not a case for refactoring, since each output
> > class can and does (XvMC and ivtv) need to handle this differently.
>
> It's not obvious at all.
>
> <soapbox>
> Any time there's code repetition it's a case for refactoring.  If
> there's a "style of pausing" that's used by multiple VideoOutput
> subclasses, and it requires updating the scratch frame's frame number
> in some particular way, it should have been factored out.  Had that
> been done properly, this bug would never have appeared in the Mac
> frontend without biting everyone else, too.  So it would have been
> noticed and fixed immediately.  And of course, that's the
> whole point of refactoring to eliminate code duplication.
> </soapbox>

No.  We're talking about a single line of code duplication in a couple of  
child classes but not others, and which is dependant on the manner of 
operation in another of the sibling classes.

Do you really think that a few hundred lines of email are necessary for a one 
line change for a simple bug?  Seems rather silly to me.  Make a tiny little 
patch, create a ticket in trac, attach it.  Much easier.

Isaac