[mythtv] [PATCH] Control duplicates by channel ID

[David Shay]

On 8/5/07, David Shay <david at shay.net> wrote:
> On 8/5/07, David Engel <david at istwok.net> wrote:
> > On Sun, Aug 05, 2007 at 07:42:42AM -0500, David Shay wrote:
> > > Determined what at least one of the problems here is.  The primary key
> > > to oldrecorded does not include chanid. This makes the "REPLACE"
> > > statement in AddHistory not add a new record when it now should.  I'm
> > > not sure if there would be any other impacts of just adding chanid
> > > into the key for oldrecorded.
> >
> > Oh, shoot.  Yes, oldrecorded imposes the restriction of one entry per
> > title, starttime and callsign.  The scheduler relies on this when
> > restoring any previous status for programs that aren't currently
> > recording and for catching reactivation requests.  I'd be very
> > concerned about the impact of allowing additional entries per chanid
> > on this functionality.
> >
>
> Seems like oldrecorded primarily gets updated by the start of
> RunScheduler, where everything gets set to rsAborted, and then
> subsequent calls to AddHistory.
>
> I've done a quick analysis of where AddHistory is called within
> scheduler.cpp. (The only one outside is in tv_rec.cpp, and it's OK).
> Here's where it's called:
>
> UpdateRecStatus (the pointer to ProgramInfo version)
>    Could be changed to use IsSameProgram instead of
>    IsSameProgramTimeslot and this might be OK?
>
> UpdateRecStatus(cardid, chanid,startts,recstatus,endts)
>    Already being qualified by chanid check, should be OK?
>
> SlaveConnected
>    First two calls are being qualified by cardid already.  This happens
>    in many later calls, as well.  My assumption is that if it's already
>    qualified by cardid, that should be sufficient and no additional
>    qualification by chanid would be necessary.
>
>    The last call is not qualified by anything, but that seems to just be the
>    fall-through case.  I wouldn't think this one would be disturbed by the
>    index not being there, but I don't know.  In what case would that matter?
>
> SlaveDisconnected
>    This call is qualified by cardid.
>
> AddRecording
>    Was using IsSameProgramTimeslot.  I guess this should also be
>    changed to IsSameProgram?
>
> RunScheduler
>    Around line 1411 now, there is a check for if nextrecording->recstatus
>    != rsWillRecord, then AddHistory gets called.  I'm not sure from this code,
>    what status nextrecording would even get set to?
>
>    The next call is qualified by cardid.
>    The next call is based on IsTunerLocked, should be OK.
>    The last call is unqualified, but I think should be OK, since it is just in
>    the main loop through the PI records.
>
> Seems to me like even those unqualified calls are OK, and that the
> ones qualified by cardid are OK as well.
>
> ReactivateRecording within programinfo.cpp would need to get changed
> to also pass chanid as a parameter as well.  But apparently that only
> gets called from the GUI, not the scheduler.
>
> > > I'm pretty sure there are some other cases where things could go wrong
> > > as well in the calls to AddHistory, particularly where
> > > IsSameProgramTimeslot is called on a pointer to a PI structure.  There
> > > the checks don't take chanid into account.  In my current code I've
> >
> > I think IsSameProgramTimeslot is a hold over from long ago when the
> > program start and times weren't kept around and only the recording
> > times available.  The IsSameProgramTimeslot check was made fuzzy to
> > allow for differences in the recording times that could creep in
> > between the scheduler and the recorders.  It looks like
> > Scheduler::UpdateRecStatus is the only remaining user of
> > IsSameProgramTimeslot and it can probably be replaced with
> > IsSameProgram now.
>
>
> So you are saying if you had pre-roll/post-roll kinds of things,
> that's what it would be used for?  Right now, anyway,
> IsSameProgramTimeslot looks to make sure that the given program is
> wholly contained within the other program, and matches on title and
> chanid.  The only thing I'm a little concerned about is that
> IsSameProgram does no element of time checking at all.  Wouldn't that
> be a problem?
>

If this analysis is correct, attached is the latest patch that
includes the update to dbcheck.cpp.  I didn't phase out
IsSameProgramTimeslot, but just qualified it with the dupchanid and
chanid checks.

Let me know.