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

[Michael Niedermayer]

On Mon, May 10, 2010 at 10:16:20PM +0200, Janne Grunau wrote:
> 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.

i dont understand the issue
before the subtitle decoder we have some binary stream
after the subtitle decoder we have rectangles that contain bitmaps or text
possibly with formating possibly with effects.
the positions of the rectangles can be specified in pixels or relative to a
0.0-1.0 screen

i see no case in which a hierachical system with an additional rectangle
that contanins the other rectangles would make sense.
thats besides that whatever is done the API must be unambigously and clearly
be described. that is the interaction/relation of all the values and their
scale must be documented.
anyway, please elaborate on what problem you see

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100520/fb25d196/attachment.pgp>