[mythtv] [PATCH] Control duplicates by channel ID

[David Shay]

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?