[MPlayer-DOCS] Re: [PATCH] x264 encoding guide

[Diego Biurrun]

On Sat, Apr 30, 2005 at 11:41:44PM +0200, Guillaume POIRIER wrote:
> 
> +<title>Encoding with the <systemitem class="library">x264</systemitem>
> + codec</title>

Nit: Please keep the <title> on one line.

> +  The <systemitem class="library">x264</systemitem> section of 
> +  <application>MPlayer</application>'s man page is excellent; please
> +  begin by reviewing it.

It's OK if Jeff writes this in an external document, but we should not
claim ourselves that our man page is excellent.  That topic is for
others to decide.

  Please begin by reviewing the x264 section of MPlayer's man page.

should suffice IMO.

> +  This document is intended to be a supplement to the man page.

It's just a section in the documentation, not a full document, so don't
call it a full section.

> +  <listitem><para>Trading off encoding time vs. quality</para></listitem>

Somehow this sounds wrong to me, maybe some of the native speakers can
comment.  "Trading off" feels strange, I'd say "make a tradeoff" or
something similar, the "vs" sounds even stranger.

> +  For a brief explanation of what PSNR is, see
> +  <ulink url="http://en.wikipedia.org/wiki/PSNR">the Wikipedia</ulink>

Missing period at the end of a sentence.

This is not a link to Wikipedia, but to the PSNR article in Wikipedia,
thus it should have a label that says so, for example "the Wikipedia
article on PSNR".

> +  Nearly all of this guide's comments assume you're using
> +  2-pass.

We've settled on "two pass" for the docs.

> +  full-screen repetitive flashing patterns or very large temporary

fullscreen

> +  <emphasis role="bold">bframes</emphasis>:
> +  The usefulness of B-frames is questionable in most other codecs
> +  you may be used to.
> +  In H.264, this has changed: there are new techniques and block
> +  types that are possible in B-frames.
> +  Usually, even a naive bframe choice algorithm can have a
> +  significant PSNR benefit.
> +  It is also interesting to note that if you turn off the adaptive
> +  b-frame decision (<option>nob_adapt</option>), encoding with
> +  bframes usually speeds up encoding speed somewhat.

B-frame is misspelled throughout this paragraph.

> +  If you're going to use bframes at all, consider setting the
> +  maximum number of bframes to 2 or higher in order to take
> +  advantage of weighted prediction.

same here

> +  With this option enabled, the encoder will use some simple
> +  heuristics to reduce the number of bframes used in scenes that
> +  might not benefit from them as much.
> +  You can use <option>b_bias</option> to tweak how b-frame-happy
> +  the encoder is.
> +  The speed penalty of adaptive bframes is currently rather modest,
> +  but so is the potential quality gain.
> +  It usually doesn't hurt, however.

and here

> +  Note that these videos can't be read by libavcodec-based decoders
> +  older than about March 12, 2005.

Maybe add the build number?

> +  of expensive I-frames; using weighted prediction in bframes

B-frames

I think it should always be either B-frames or <option>bframes</option>.

> +  I think this is almost never a good idea, and in my experience,
> +  people who are doing this don't understand very well how
> +  deblocking works by default.

I would say avoid the first person perspective, it's not the style of
the MEncoder documentation.

> +  If your H.264 encodes look too blurry or smeared, try playing with
> +  <option>-vf noise</option> when you play your encoded movie back.

I'd omit the "back" here.

> +<title>How can I play back H.264 videos with <application>MPlayer</application>?</title>

same here

> +  Just to be certain, it's always a good idea to use a recent cvs

CVS

> +  H.264 decoding, you might keep an eye on 
> +  <ulink url="http://mplayerhq.hu/cgi-bin/cvsweb.cgi/ffmpeg/libavcodec/h264.c?cvsroot=FFMpeg">FFmpeg CVS repository's web interface</ulink>.

on the


Looks good overall.  Please replace short forms by long forms everywhere.

Diego