Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
DVR: Fix the retention condition 
Introduced by "[htsp] allow clients to get/set dvr priority and retention"
  • Loading branch information
@perexg
perexg committed Sep 16, 2014
1 parent 5484e99 commit d11c919
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/dvr/dvr_db.c
View file
Expand Up @@ -112,7 +112,7 @@ dvr_entry_get_mc( dvr_entry_t *de )
int
dvr_entry_get_retention( dvr_entry_t *de )
{
if (de->de_retention >= 0)
if (de->de_retention > 0)
return de->de_retention;
return de->de_config->dvr_retention_days;
}
Expand Down

8 comments on commit d11c919

@ksooo
Copy link
Contributor

@ksooo ksooo commented on d11c919 Sep 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure whether this is the right approach.

The question is what a retenetion of zero means. Is it "not set" or "expire immediately"? Currently this two really very different values cannot be distinguished.

Would be a better approach to define DVR_RETENTION_NOT_SET as -1 and to default de_retention to this value? Makes things really clear.

@perexg
Copy link
Contributor Author

@perexg perexg commented on d11c919 Sep 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not perfect, I know. Someone might improve it, but the problem is that the default value is zero which expired all finished/failed entries . Note that the expire immediately behaviour can be configured using the configuration profile, so it's no regression.

@EricV
Copy link
Contributor

@EricV EricV commented on d11c919 Sep 16, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ksooo
Copy link
Contributor

@ksooo ksooo commented on d11c919 Sep 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EricV sure. Intention is to restore the old behavior, not to change it.

@perexg
Copy link
Contributor Author

@perexg perexg commented on d11c919 Sep 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ksooo : It's not about the old behaviour, because this per-dvr-entry retention config was added in last days and caused regressions.

@ksooo
Copy link
Contributor

@ksooo ksooo commented on d11c919 Sep 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@perexg . Absolutely. With "old behavior" I ment to remove recording entries when the recordings are actually expired according to user's preferences - short, to fix the regression introduced last week.

@EricV
Copy link
Contributor

@EricV EricV commented on d11c919 Sep 16, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ksooo
Copy link
Contributor

@ksooo ksooo commented on d11c919 Sep 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EricV : Yes, no change planned. Remove recording entries, leave corresponding files (mkv, ts2) on disk.

Please sign in to comment.