Isaac Richards <[EMAIL PROTECTED]> writes: > On Saturday 28 January 2006 22:11, David Abrahams wrote: >> Isaac Richards <[EMAIL PROTECTED]> writes: >> >> > 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?
I'd still like to know what's broken by the change I proposed. It would help me to understand the code and not waste as much of your time the next time around. > Check how it's done in VideoOutputXv::UpdatePauseFrame. Obviously, it works > there. Simple grepping would have found that. Simple grepping only works when you know what to look for. It wasn't obvious to me that VideoOutputQuartz was supposed to be following a pattern from VideoOutputXv. >> > 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, 3 lines. > and which is dependant on the manner of operation in another of the > sibling classes. Yep. Refactor mercilessly. > 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. I need at least enough lines of email to understand the problem and what the correct fix is. So far, that is only beginning to dawn on me. > Make a tiny little patch, create a ticket in trac, attach it. Much > easier. I'm doing my best. -- Dave Abrahams Boost Consulting www.boost-consulting.com _______________________________________________ mythtv-dev mailing list [email protected] http://mythtv.org/cgi-bin/mailman/listinfo/mythtv-dev
