[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 03:06:32PM -0400, David Shay wrote:
> > UpdateRecStatus (the pointer to ProgramInfo version)
> >     Could be changed to use IsSameProgram instead of
> >     IsSameProgramTimeslot and this might be OK?
>
> See below.
>
> > 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?
>
> Yes for all of these, I think, as far as IsSameProgram is concerned.
> Not related to this, however, I think some of those cardid checks
> should be cardid and inputid checks.  It's probably highly unlikely,
> but I suppose it is possible the same chanid could be on multiple
> inputs on the same card.
>
> > 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?
>
> Is that the case which is also qualified by recstatus != oldrecstatus?
> If so that's to catch status changes when a program is reactivated and
> possibly some others I can't remember off the top of my head.

Yes, that's the one.  The thing that was confusing is that in most
cases, before AddHistory is called, recstatus is set to something.  In
this case, it is just called without setting recstatus at all.


>
> > 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 think you're missing the other use of oldrecorded that I mentioned
> earlier.  Please see the oldrecstatus join in the scheduler query.
>

Right, I hadn't seen the oldrecstatus join, only the main oldrecorded
join.  See below for other comments.

> When a reschedule is done, the scheduler only keeps entries for
> currently recording programs in memory.  The scheduler re-reads the
> state for any programs which are currently being shown but have
> already been dispositioned from the oldrecorded table.  Reactivation
> requests are also pulled in in this way.
>
> The old state and reactivation values come from the oldrecstatus join
> in the scheduler query.  This join intentionally only uses callsign
> because, until now, with the exception actually choosing which card,
> input and chanid to record on, the same program from the same station
> was always treated the no matter what chanid it was on.
>
> Additionally, PL::FromProgram uses a similar join to read the status
> of past programs the scheduler no longer cares about.  This is how the
> EPG can show the recording status for yesterday's programs.
>
> It is the impact of having multiple chanid entries in oldrecrded on
> these uses that I am concerned about.  If nothing special is done,
> those queries could start returning duplicate results.  Perhaps any
> duplicates would be harmless and everything would come out in the
> wash.  I don't know yet.
>
> OK, so what do we do?
>
> ONe option is to do away with all of the coalescing we do on callsign
> and callsign/channel number.  ViewScheduled would always show multiple
> entries for the same program on different chanids, even if the channel
> number is the same.  Likewise, oldrecorded would always show multiple
> entries.
>
> Change the scheduler to make the oldrecstatus join condition smarter
> to use chanid when applicable and not when it isn't.

That seems like the right/easiest thing to do within the scheduler,
since we already have the information and it's consistent with the
other joins to oldrecorded and recorded.

>I'm not sure
> PL::FromProgram could do this since the recordid wouldn't be available
> until after the join.

Right.  However, Bruce earlier added code to insert the additional
check after the query to verify on dupchanid and check the chanid's.
That still doesn't guarantee the right status for programs in the
past, though.  What about "hardcoding" a forced join on chanid, just
for FromProgram?

>
> > So you are saying if you had pre-roll/post-roll kinds of things,
> > that's what it would be used for?
>
> Any number of things -- pre/post-roll, start-early/end-late, not
> asking to record a record a program until after it's started,
> reactivating a stopped program.
>
> > 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.
>
> Not wholly contained, just overlapping.
>
> > 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?
>
> Now it's my turn to be missing something! :( I was completely wrong
> before in saying IsSameProgramTimeslot could be replaced with
> IsSameProgram.  The should be replaced with IsSameTimeslot.  My bad.
>

That makes more sense. Gee, such a simple topic, not sure how anyone
could get confused :)