Mailing List Archive

Services bug with dupin field
MythTV Devs

The dupin (Dup Scope) field in the record table is used for telling
whether to check for duplicates in Recorded, Oldrecorded, or both. It is
also used to indicate whether to record only "new episodes" or "new and
repeat episodes". The value for "new episodes" is 0x10 and that is
"ored" with the other duplicate settings (0x01, 0x02, 0x0f). The
toRawString and other similar functions in recordingtypes.cpp assign the
value "New Episodes Only" to the value 16 but that is never found, as
the 0x10 flag is always set in conjunction with other values, so if you
have "Record New Episodes Only" selected, the services apis
GetRecordSchedule and GetRecordScheduleList return "Unknown" for Dupin.

I propose changing the toRawString and similar routines to return the
value "New Episodes Only" when the 0x10 bit is set, regardless of other
values, since checking for duplicates is moot if you are recording new
episodes only, and "Unknown" is not helpful.

In the case of the "AddRecordSchedule" method, setting "New Episodes
Only" will currently set a value of 0x10. This is invalid since creation
of record rules through the frontend never results in this value. In the
frontend you always have to also set where to check duplicates, you
cannot set it to check duplicates in neither recorded nor oldrecorded
(even if you selected "Dont Match Duplicates" in the dup method)

I propose changing dupInFromString so that if you set "new episodes
only" it combines that with "all recordings".  Otherwise if you create a
record rule through the api using "new episodes only" and subsequently
edit it through the frontend to take off "new episodes only" you may be
left with a rule having an invalid dupin of zero.

Any comments or suggestions? Let me know.

The frontend is a bit inconsistent - if you select "don't match
duplicates" it prevents you from changing "dup scope" but if you
selected "new episodes only" you can still select duplicate checking
methods and scope.

Peter


_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org
Re: Services bug with dupin field [ In reply to ]
On Fri, Aug 21, 2020 at 02:48:27PM -0400, Peter Bennett wrote:
> MythTV Devs
>
> The dupin (Dup Scope) field in the record table is used for telling whether
> to check for duplicates in Recorded, Oldrecorded, or both. It is also used
> to indicate whether to record only "new episodes" or "new and repeat
> episodes". The value for "new episodes" is 0x10 and that is "ored" with the
> other duplicate settings (0x01, 0x02, 0x0f). The toRawString and other
> similar functions in recordingtypes.cpp assign the value "New Episodes Only"
> to the value 16 but that is never found, as the 0x10 flag is always set in
> conjunction with other values, so if you have "Record New Episodes Only"
> selected, the services apis GetRecordSchedule and GetRecordScheduleList
> return "Unknown" for Dupin.
>
> I propose changing the toRawString and similar routines to return the value
> "New Episodes Only" when the 0x10 bit is set, regardless of other values,
> since checking for duplicates is moot if you are recording new episodes
> only, and "Unknown" is not helpful.

That's not correct. New Episodes Only is *not* a filter like the New
Episodes filter. Rules using thie New Episodes Only still match all
episodes. However, if a program is not new(*), it is marked with
RecStatus::Repeat. In contrast, the New Episode filter doesn't match
repeat programs at all.

(*)"New" is determined by the repeat flag. Originally, this was
defined as something like within 2 weeks of the original air date.
The details might have changed over the years, but the intent is still
the same. Anyway, duplicate checking is still needed when New
Episodes Only is used as many channels repeat new programs one or more
times the first week they are shown.

> In the case of the "AddRecordSchedule" method, setting "New Episodes Only"
> will currently set a value of 0x10. This is invalid since creation of record
> rules through the frontend never results in this value. In the frontend you
> always have to also set where to check duplicates, you cannot set it to
> check duplicates in neither recorded nor oldrecorded (even if you selected
> "Dont Match Duplicates" in the dup method)
>
> I propose changing dupInFromString so that if you set "new episodes only" it
> combines that with "all recordings".? Otherwise if you create a record rule
> through the api using "new episodes only" and subsequently edit it through
> the frontend to take off "new episodes only" you may be left with a rule
> having an invalid dupin of zero.
>
> Any comments or suggestions? Let me know.

I think the best solutino is to split the new episodes only/new and
repeat value out into a totally new value. The trick will be to
maintain as much backward compatibility as you can.

> The frontend is a bit inconsistent - if you select "don't match duplicates"
> it prevents you from changing "dup scope" but if you selected "new episodes
> only" you can still select duplicate checking methods and scope.

The overloading of dupscope to include the new/repeat functionality
was an unfortunate decision. I was around then but don't remember why
it was done that way.

David
--
David Engel
david@istwok.net
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org
Re: Services bug with dupin field [ In reply to ]
On 8/21/20 9:09 PM, David Engel wrote:
> On Fri, Aug 21, 2020 at 02:48:27PM -0400, Peter Bennett wrote:
>> MythTV Devs
>>
>> The dupin (Dup Scope) field in the record table is used for telling whether
>> to check for duplicates in Recorded, Oldrecorded, or both. It is also used
>> to indicate whether to record only "new episodes" or "new and repeat
>> episodes". The value for "new episodes" is 0x10 and that is "ored" with the
>> other duplicate settings (0x01, 0x02, 0x0f). The toRawString and other
>> similar functions in recordingtypes.cpp assign the value "New Episodes Only"
>> to the value 16 but that is never found, as the 0x10 flag is always set in
>> conjunction with other values, so if you have "Record New Episodes Only"
>> selected, the services apis GetRecordSchedule and GetRecordScheduleList
>> return "Unknown" for Dupin.
>>
>> I propose changing the toRawString and similar routines to return the value
>> "New Episodes Only" when the 0x10 bit is set, regardless of other values,
>> since checking for duplicates is moot if you are recording new episodes
>> only, and "Unknown" is not helpful.
> That's not correct. New Episodes Only is *not* a filter like the New
> Episodes filter. Rules using thie New Episodes Only still match all
> episodes. However, if a program is not new(*), it is marked with
> RecStatus::Repeat. In contrast, the New Episode filter doesn't match
> repeat programs at all.
>
> (*)"New" is determined by the repeat flag. Originally, this was
> defined as something like within 2 weeks of the original air date.
> The details might have changed over the years, but the intent is still
> the same. Anyway, duplicate checking is still needed when New
> Episodes Only is used as many channels repeat new programs one or more
> times the first week they are shown.
OK I understand now.
>> In the case of the "AddRecordSchedule" method, setting "New Episodes Only"
>> will currently set a value of 0x10. This is invalid since creation of record
>> rules through the frontend never results in this value. In the frontend you
>> always have to also set where to check duplicates, you cannot set it to
>> check duplicates in neither recorded nor oldrecorded (even if you selected
>> "Dont Match Duplicates" in the dup method)
>>
>> I propose changing dupInFromString so that if you set "new episodes only" it
>> combines that with "all recordings".  Otherwise if you create a record rule
>> through the api using "new episodes only" and subsequently edit it through
>> the frontend to take off "new episodes only" you may be left with a rule
>> having an invalid dupin of zero.
>>
>> Any comments or suggestions? Let me know.
> I think the best solutino is to split the new episodes only/new and
> repeat value out into a totally new value. The trick will be to
> maintain as much backward compatibility as you can.

So, change the service apis so that Dupin returns only "Current
Recordings" , "Previous Recordings" or "All Recordings" by first
removing the "New episodes" flag if present. Then add a new tag called
"NewEpisodesOnly" with boolean true/false value, based on that flag.

The new tag should not affect existing users who should just ignore it.

In the AddRecordSchedule method if they do submit value "New Episodes
Only" for Dupin and leave out the "NewEpisodesOnly" tag, then continue
with what it would have done before, set only that value with nothing
for current, previous or all recordings (for backward compatibility).

Does this sound good?

Peter


_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org
Re: Services bug with dupin field [ In reply to ]
On Sat, Aug 22, 2020 at 11:16:11AM -0400, Peter Bennett wrote:
>
> On 8/21/20 9:09 PM, David Engel wrote:
> > On Fri, Aug 21, 2020 at 02:48:27PM -0400, Peter Bennett wrote:
> > > Any comments or suggestions? Let me know.
> > I think the best solutino is to split the new episodes only/new and
> > repeat value out into a totally new value. The trick will be to
> > maintain as much backward compatibility as you can.
>
> So, change the service apis so that Dupin returns only "Current Recordings"
> , "Previous Recordings" or "All Recordings" by first removing the "New
> episodes" flag if present. Then add a new tag called "NewEpisodesOnly" with
> boolean true/false value, based on that flag.
>
> The new tag should not affect existing users who should just ignore it.
>
> In the AddRecordSchedule method if they do submit value "New Episodes Only"
> for Dupin and leave out the "NewEpisodesOnly" tag, then continue with what
> it would have done before, set only that value with nothing for current,
> previous or all recordings (for backward compatibility).
>
> Does this sound good?

Mostly. If no information for current/previous recordings is provided
(i.e. new episodes only in dupin and no new tag), set both current and
previous recordings in dupin. That "fixes" the old behavior as I
consider it a bug.

David
--
David Engel
david@istwok.net
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org
Re: Services bug with dupin field [ In reply to ]
On 8/22/20 1:07 PM, David Engel wrote:
> On Sat, Aug 22, 2020 at 11:16:11AM -0400, Peter Bennett wrote:
>> On 8/21/20 9:09 PM, David Engel wrote:
>>> On Fri, Aug 21, 2020 at 02:48:27PM -0400, Peter Bennett wrote:
>>>> Any comments or suggestions? Let me know.
>>> I think the best solutino is to split the new episodes only/new and
>>> repeat value out into a totally new value. The trick will be to
>>> maintain as much backward compatibility as you can.
>> So, change the service apis so that Dupin returns only "Current Recordings"
>> , "Previous Recordings" or "All Recordings" by first removing the "New
>> episodes" flag if present. Then add a new tag called "NewEpisodesOnly" with
>> boolean true/false value, based on that flag.
>>
>> The new tag should not affect existing users who should just ignore it.
>>
>> In the AddRecordSchedule method if they do submit value "New Episodes Only"
>> for Dupin and leave out the "NewEpisodesOnly" tag, then continue with what
>> it would have done before, set only that value with nothing for current,
>> previous or all recordings (for backward compatibility).
>>
>> Does this sound good?
> Mostly. If no information for current/previous recordings is provided
> (i.e. new episodes only in dupin and no new tag), set both current and
> previous recordings in dupin. That "fixes" the old behavior as I
> consider it a bug.
>
> David

OK, done.

I changed the new tag to "NewEpisOnly" because "NewEpisodesOnly" is a
longer name than any other tag.

See attached patch, lightly tested on GetRecordScheduleList, not tested
on update or add. More testing needed before committing it.

There are unresolved bugs in recordingtypes.cpp in
toString(RecordingDupInType) and toDescription(RecordingDupInType) which
will not return correct results when "New Episodes Only" is selected. I
don't know how these are used, I could change them to combine the two
values (for example "All Recordings, New Episodes Only"), but I don't
know if that is appropriate. The caller may not expect that.

Peter
Re: Services bug with dupin field [ In reply to ]
On Sat, Aug 22, 2020 at 03:00:29PM -0400, Peter Bennett wrote:
> There are unresolved bugs in recordingtypes.cpp in
> toString(RecordingDupInType) and toDescription(RecordingDupInType) which
> will not return correct results when "New Episodes Only" is selected. I
> don't know how these are used, I could change them to combine the two values
> (for example "All Recordings, New Episodes Only"), but I don't know if that
> is appropriate. The caller may not expect that.

I know the schedule editor properly presents the values separately.
I'm not sure what else might use those funtions. We'll probably have
to audit each call to find out.

David
--
David Engel
david@istwok.net
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org