[mythtv] [PATCH] fix auto-transcode for "find once" schedules
Robert Tsai
rtsai1111 at comcast.net
Tue Jun 21 14:13:40 UTC 2005
On Tue, Jun 21, 2005 at 02:15:53AM -0400, Chris Pinkham wrote:
> Can you take a look at the attached patch and see if it looks like
> it accomplishes the same thing you were going for. I have had a
> couple complaints about the same bug with commercial flagging and
> hadn't had a chance to investigate yet.
Your patch works for me (I tested it on a garbage 5-minute "find-once"
recording this morning).
> I think this makes the code a little cleaner and fixes the bug you
> were trying to fix as well as the one I needed to fix. :)
I agree. I have some small comments:
> +#define ClearJobMask(mask) mask = JOB_NONE;
> +#define JobIsInMask(job, mask) ((bool)(job & mask))
> +#define JobIsNotInMask(job, mask) (!(JobIsInMask(job, mask)))
> +#define AddJobsToMask(jobs, mask) mask |= jobs;
> +#define RemoveJobsFromMask(jobs, mask) mask &= (~jobs);
> +
The macro args should (always, IMHO) be parenthesized. Otherwise,
things like:
JobIsInMask(job, JOB_TRANSCODE | JOB_COMMFLAG)
RemoveJobsFromMask(jobs, JOB_TRANSCODE | JOB_COMMFLAG)
will not work as expected, because they will have textually expanded
to become:
((bool)(job & JOB_TRANSCODE | JOB_COMMFLAG))
jobs &= (~JOB_TRANSCODE | JOB_COMMFLAG)
which is probably not what the programmer wants ('&' and '~' bind more
tightly than '|').
Another option would be to just use subroutines (inlined, if the
performance of these statements really really matters to you).
> +/* vim: set expandtab tabstop=4 shiftwidth=4: */
Heh. This is good :).
> - // Determine whether to automatically run the transcoder or not
> - Setting *profileAutoTranscode = profile.byName("autotranscode");
> - autoTranscode = (profileAutoTranscode &&
> - profileAutoTranscode->getValue().toInt() != 0) ?
> - true : false;
> + ClearJobMask(autoRunJobs);
> + if ((tmpInternalState != kState_WatchingLiveTV) &&
> + (curRecording))
> + {
> + AddJobsToMask(curRecording->GetAutoRunJobs(), autoRunJobs);
> +
> + // Make sure transcoding is OFF if the profile does not allow
> + // AutoTranscoding.
> + Setting *profileAutoTranscode = profile.byName("autotranscode");
> + if ((!profileAutoTranscode) ||
> + (profileAutoTranscode->getValue().toInt() == 0))
> + RemoveJobsFromMask(JOB_TRANSCODE, autoRunJobs);
// Don't commflag commercial-free recordings
if (!curRecording->chancommfree)
RemoveJobsFromMask(JOB_COMMFLAG, autoRunJobs);
> + }
>
> bool error = false;
>
> @@ -705,16 +713,18 @@
> frameRate = recorder->GetFrameRate();
>
> if ((tmpInternalState != kState_WatchingLiveTV) &&
> - (!curRecording->chancommfree) &&
> - (curRecording->GetAutoRunJobs() & JOB_COMMFLAG) &&
> - (earlyCommFlag) &&
> - ((autoTranscode && !transcodeFirst) || (!autoTranscode)))
> + (curRecording) && (!curRecording->chancommfree) &&
> + (JobIsInMask(JOB_COMMFLAG, autoRunJobs)) && (earlyCommFlag) &&
> + ((JobIsNotInMask(JOB_TRANSCODE, autoRunJobs)) ||
> + (JobIsInMask(JOB_TRANSCODE, autoRunJobs) && !transcodeFirst)))
Would it make sense to also examine curRecording->chancommfree earlier
(see above)?
That way this big "if" can be reduced to just examining the order in
which transcoding/commflagging should take place:
if (tmpInternalState != kState_WatchingLiveTV &&
curRecording &&
JobIsInMask(JOB_COMMFLAG, autoRunJobs) &&
(earlyCommFlag ||
JobIsNotInMask(JOB_TRANSCODE, autoRunJobs) ||
!transcodeFirst))
> {
> JobQueue::QueueJob(
> JOB_COMMFLAG, curRecording->chanid,
> curRecording->recstartts, "", "",
> (runJobOnHostOnly) ? gContext->GetHostName() : "",
> JOB_LIVE_REC);
> +
> + RemoveJobsFromMask(JOB_COMMFLAG, autoRunJobs);
> }
> }
> else
--Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://mythtv.org/pipermail/mythtv-dev/attachments/20050621/a82865b9/attachment.pgp
More information about the mythtv-dev
mailing list