[FFmpeg-devel] [PATCH] Add support for parsing the Display Definition Segment in dvbsubdec.c

[Janne Grunau]

On Fri, May 07, 2010 at 12:50:05AM +0200, Michael Niedermayer wrote:
> On Thu, May 06, 2010 at 01:11:58AM +0200, Janne Grunau wrote:
> > On Fri, Apr 23, 2010 at 03:41:09PM +0200, Michael Niedermayer wrote:
> > > 
> > > agree, 
> > > with documentation added to width/height and where the x/y position is
> > > documented
> > 
> > modified patch attached. Also moved display_x/y to AVSubtitle since it's
> > at least in the case of dvb subtitles for all rects identical. If we
> > expect it to change per rect we can't store the size in AVCodecContext.
> > Also bumped minor version, removed whitespace only changes and fixed
> > off-by-one errors in the width/height calculation.
> > 
> >  avcodec.h   |   10 ++++++++-
> >  dvbsubdec.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 73 insertions(+), 1 deletion(-)
> > dcde49dfda3c601c37db785edef94505562ea1f2  hd_dvb_subtitles2.diff
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index b643747..0abd53f 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -30,7 +30,7 @@
> >  #include "libavutil/avutil.h"
> >  
> >  #define LIBAVCODEC_VERSION_MAJOR 52
> > -#define LIBAVCODEC_VERSION_MINOR 66
> > +#define LIBAVCODEC_VERSION_MINOR 67
> >  #define LIBAVCODEC_VERSION_MICRO  0
> >  
> >  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> > @@ -1097,6 +1097,9 @@ typedef struct AVCodecContext {
> >       * picture width / height.
> >       * - encoding: MUST be set by user.
> >       * - decoding: Set by libavcodec.
> > +     * If set for subtitles width / height of the display region. The top
> > +     * left corner of the display region is specified in AVSubtitle as
> > +     * display_x, display_y.
> >       * Note: For compatibility it is possible to set this instead of
> >       * coded_width/height before decoding.
> >       */
> 
> > @@ -2863,6 +2866,11 @@ typedef struct AVSubtitle {
> >      unsigned num_rects;
> >      AVSubtitleRect **rects;
> >      int64_t pts;    ///< Same as packet pts, in AV_TIME_BASE
> > +    /**
> > +     * top left corner of region into which rects are displayed.
> > +     * display width and height are in AVCodecContext
> > +     */
> > +    int display_x, display_y;
> 
> the rectangles already contain x/y variables this is unneeded and
> confusing

unfortunately the specs are confusing and overly complicated. see Section 7.2.1
and Annex B of
http://www.etsi.org/deliver/etsi_en/300700_300799/300743/01.03.01_60/en_300743v010301p.pdf

Especially point b) of Annex B doesn't make much sense.

We could add the x/y from the display window to the values in the rects
but it would make scaling the subtitles much harder.

Janne