[FFmpeg-devel] [PATCH 1/2] Modified to generate PAT/PMT on every video keyframe. This is so that TS fragments produced by http://code.google.com/p/httpsegmenter/ would be compatible with JW Player.

[Reimar Döffinger]

On Fri, Feb 10, 2012 at 01:03:02PM -0700, Pavel Koshevoy wrote:
> On 2/10/2012 12:29 PM, Reimar Döffinger wrote:
> >On Fri, Feb 10, 2012 at 12:06:36PM -0700, pkoshevoy at gmail.com wrote:
> >
> >Please format the commit message so that after a short summary
> >you have an empty line and then details.
> >The short summary should be preferably be max. 60 characters to
> >nicely fit into the email subject.
> 
> Yes, sorry about the long subject line.  This is the first time I am
> using git or submitting to ffmpeg-devel.  I am used to writing
> descriptive paragraphs for CVS/SVN commits.  I didn't know git
> format-patch and git send-email would do what they did.

Yes, it happened to most of us at the start, that's why
I gave the explanation.

> >>@@ -788,10 +788,12 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
> >>      int afc_len, stuffing_len;
> >>      int64_t pcr = -1; /* avoid warning */
> >>      int64_t delay = av_rescale(s->max_delay, 90000, AV_TIME_BASE);
> >>+    int force_pat = key&&  (st->codec->codec_type == AVMEDIA_TYPE_VIDEO);
> >The () are unnecessary.
> 
> Yes, () are unnecessary from compilers perspective, but they do
> enhance readability for humans (at least in this instance in my
> opinion).  Is this a rejection then?

Generally I think we avoid it in FFmpeg (when it is == and &&
I personally think it makes it less readable - even more
so if it does not match the rest of FFmpeg style).
But that's not so important, I just thought I should write
a bit more than just a git-howto, listen to Michael mostly :-)
Btw.  if you don't know it yet, tools/patcheck is a nice tool
to run on patches (though I think it does not yet detect a badly
formatted commit message).

> >I don't know enough about TS to comment much, except that this
> >is possibly a bit crazy for the intra-only use case.
> 
> 
> Yes, this would add some overhead in intra-only case, but that would
> not break compatibility.  When intra-only is used does anyone really
> care about file size?

No idea, that's why I wrote "possibly a bit crazy" ;-)