[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