Project

General

Profile

Feature #874

Extra time before recordings - negative value is stored incorrectly

Added by Tomas Matejicek almost 13 years ago. Updated over 7 years ago.

Status:
Fixed
Priority:
Low
Assignee:
Adam Sutton
Category:
PVR / DVR
Target version:
-
Start date:
2012-02-10
Due date:
% Done:

0%

Estimated time:
0.10 h

Description

This bug can be fixed very easily.
I tried to store a negative value (like -10) for Extra time before recordings (preExtraTime) through the webGUI of tvheadend.
This can be useful to start recording later than the EPG claims it starts.
Unfortunately there is a function call htsmsg_add_u32() and htsmsg_get_u32() which handles this parameter on several places, making negative value overflow due to signed/unsigned integer problem.

To fix this, you just have to use htsmsg_add_s32() and htsmsg_get_s32() everywhere where you save/restore settings for preExtraTime, and preferably also for postExtraTime. That's very quick fix in just few files and few lines.

That's it! This tiny fix will add great feature (to postpone recording)

History

#1

Updated by Adam Sutton over 12 years ago

  • Tracker changed from Bug to Feature
  • Status changed from New to Rejected
  • Estimated time changed from 0.10 h to 0.10 h

I'm going to reject this on the basis that I can't see why you'd want to miss the start of a show? If you still think this is a good idea and can give me a use case (just to humour me ;) ), re-submit and I'll take a look.

Even better, provide a patch :)

#2

Updated by Tomas Matejicek over 12 years ago

Well, I don't know if that amuses you, but there is a fair reason - commercials.
It is pretty common that movies on some channels start 10 minutes later due to commercials.
Some channels are famous to never start the movie on time, but always 10 minutes later.
Thus using a value of -10 for preExtraTime would save me some disk space and extra seeking on all movies.

#3

Updated by Adam Sutton over 12 years ago

  • Status changed from Rejected to Accepted
  • Estimated time changed from 0.10 h to 0.10 h

Hey Tomas,

Sorry if I caused any offence, I've been wading through a LOT of issues (to clear stuff out) and was trying to add a touch of humour (to keep myself sane).

I couldn't understand what you were trying to achieve and it wasn't clear from your original submission. However I can understand the "long set of commercials before show" problem I have seen it myself, just never thought to approach it like this.

I'll need to look at this closely (in case there are any unintended side effects), but assuming there aren't I think we can get this included. But if you do fancy creating a patch or submitting a pull request on github, that would save me some effort (as I'm pretty snowed under at the moment sorting things out) and be very much appreciated.

Regards
Adam

#4

Updated by Tomas Matejicek over 12 years ago

Actually I'm not so good in coding :) I found that the problem with negative time being stored incorrectly is caused by the *u32 and *s32 variants of the store/restore functions, but to be honest I have no idea if the negative value could break some other stuff. Furthermore I tried to code it myself with only partial success, the value was sometimes stored incorrectly anyway. So I guess I'll let it up to you, I'm sorry for being so unhlepful.

#5

Updated by Adam Sutton over 12 years ago

  • Estimated time changed from 0.10 h to 0.10 h

You'll have to leave it with me then. The main issue is that those values start/stop pre/post are littered throughout the code in several different areas (DVR, HTSP, WebUI, etc..) and they all need to be updated to ensure the types match. Not a difficult job, but tedious and time consuming.

Then there is reviewing whether it actually works. But I do accept there is merit in the request and I don't think its that likely it should cause problems (but it needs to be checked).

I can't promise anything on time scales unfortunately as there are higher priority tasks to complete and just generally getting the project back on track.

Adam

#6

Updated by Adam Sutton over 12 years ago

  • Category set to PVR / DVR
  • Assignee set to Adam Sutton
  • Priority changed from Normal to Low
  • Estimated time changed from 0.10 h to 0.10 h
#7

Updated by Jaroslav Kysela over 7 years ago

  • Status changed from Accepted to Fixed

I believe that this is fixed in 4.x.

Also available in: Atom PDF