headers
Uoti Urpala | 1 Dec 2007 01:00
Picon
Picon

Re: [FFmpeg-cvslog] r11100 - in trunk/libavcodec/i386: cavsdsp_mmx.c dsputil_mmx.c dsputil_mmx.h h264dsp_mmx.c mpegvideo_mmx.c vc1dsp_mmx.c

On Fri, 2007-11-30 at 21:11 +0100, Michael Niedermayer wrote:
> On Fri, Nov 30, 2007 at 07:04:42PM +0200, Uoti Urpala wrote:
> > Any linker stuff like (3) or (4) cannot give the full available
> speedup;
> 
> yes, but adding -Bsymbolic to ld is easy, getting the vissibility correct
> migh not be, i dunno, iam happy with a clean patch which fixes the vissibility
> of symbols to the minimum overrideable and vissible as well
> still i think (3) is simpler ...

Getting visibility correct is very straightforward and obvious if you do
it per symbol, but requires a code change for each symbol whose
visibility you want to change. Basically you'd have 'static' for things
visible only in the same translation unit, and 'FF_PRIVATE' (or some
better macro name) for things visible only in the same library. Pragmas
allow getting the result with much fewer code changes but result in less
obvious code.

-Bsymbolic is "simpler" of course but it does not achieve the same
things.
Permalink | Reply | 3 comments |
headers
Aurelien Jacobs | 1 Dec 2007 01:03

Re: VP5 interlaced mode, any hints?

On Fri, 30 Nov 2007 10:22:13 +0100
Reimar Döffinger <Reimar.Doeffinger <at> stud.uni-karlsruhe.de> wrote:

> Hello,
> On Fri, Nov 30, 2007 at 12:09:24AM +0100, Aurelien Jacobs wrote:
> > Reimar Döffinger wrote:
> > > this is a sample for the VP5 interlaced mode:
> > > http://samples.mplayerhq.hu/V-codecs/VP5/vp5_interlace.avi
> > > does someone have any hints what is needed to make it play right?
> > > I creates the typical artefacts that appear when you either use the
> > > wrong IDCT or use it on the wrong data...
> > 
> > I started implementing "interlaced" vp6 decoding some times ago.
> > I never finished it, but I think the code is almost OK.
> > At least bitstream parsing should be OK (even for vp5).
> > Unfortunately it don't produce much better result with your
> > vp5 file.
> > (see attached patch)
> 
> Do you have a (mostly) working sample file, preferable vp5?

IIRC, your sample is the first interlaced VP5 file I encounter.
But, still IIRC, the following VP6 sample use interlacing and
is mostly working:
http://samples.mplayerhq.hu/V-codecs/VP6/vid.avi

Aurel
Permalink | Reply | 0 comments |
headers
Aurelien Jacobs | 1 Dec 2007 01:44

Re: [RFC] ff_huff_build_tree depends on uninitialized data+

On Fri, 30 Nov 2007 19:47:06 +0100
Reimar Döffinger <Reimar.Doeffinger <at> stud.uni-karlsruhe.de> wrote:

> Hello,
> On Fri, Nov 30, 2007 at 09:14:35PM +0200, Kostya wrote:
> > On Fri, Nov 30, 2007 at 06:57:31PM +0100, Reimar Döffinger wrote:
> > > Hello,
> > > that function has the following code
> > > >    cur_node = nb_codes;
> > > >    for(i = 0; i < nb_codes*2-1; i += 2){
> > > >        nodes[cur_node].sym = HNODE;
> > > >        nodes[cur_node].count = nodes[i].count + nodes[i+1].count;
> > > >        nodes[cur_node].n0 = i;
> > > >        for(j = cur_node; j > 0; j--){
> > > >            if(nodes[j].count > nodes[j-1].count ||
> > > 
> > > 
> > > Only the first nb_codes of nodes.count must be initialized.
> > > Assume that nb_codes == 1.
> > > Then
> > > > nodes[1].count = nodes[0].count + nodes[1].count;
> > > will be executed, which is undefined.
> > > And a few lines down, nodes[1].count is compared against nodes[0].count.
> > > There are obviously load of ways to fix it, the simples being probably
> > > to do
> > > > nodes[2*nb_codes-1].count = 0;
> > > somewhere before, but I am not sure if that is correct.
> > > Could someone please look at it?
> > > I think this might be what causes the crash in the vp6 codec in issue
> > > 275.
(Continue reading)

Permalink | Reply | 1 comment |
headers
Loren Merritt | 1 Dec 2007 01:56

Re: [FFmpeg-cvslog] r11100 - in trunk/libavcodec/i386: cavsdsp_mmx.c dsputil_mmx.c dsputil_mmx.h h264dsp_mmx.c mpegvideo_mmx.c vc1dsp_mmx.c

On Fri, 30 Nov 2007, Uoti Urpala wrote:

> Would (3) or (4) actually fix compilation in this case? I think the
> addressing mode generated by the compiler causes the problems and AFAIK
> the linker wouldn't change that.

But this addressing is not generated by the compiler, hence the problem.

Static library: inline asm contains a textrel. ok.
Shared library: inline asm still contains a textrel. linker barfs because 
that's not PIC.
Shared library with -Bsymbolic: inline asm still contains a textrel. 
linker resolves the textrel when making the shared library, thus there 
are no textrels in the .so, so it's ok.

> Any linker stuff like (3) or (4) cannot give the full available speedup;
> for that the addressing mode needs to be known at compile time. As I
> wrote in another message the visibility should be marked in the headers
> with either a pragma changing default visibility or a per-symbol macro.
> Those produce identical results but differ in code maintenance
> qualities.

Benchmarks (of x264 not ffmpeg, but they should still be relevant):

x86_64 (Core2)
speed  exe size  version
-2.0%   698928   shared
-1.3%   693584   shared -Bsymbolic
-1.3%   673136   shared -fvisibility=hidden
-0.9%   673104   shared -Bsymbolic -fvisibility=hidden
(Continue reading)

Permalink | Reply | 9 comments |
headers
Michael Niedermayer | 1 Dec 2007 02:27
Picon
Picon

Re: [PATCH] RV30/40 decoder

On Sun, Nov 18, 2007 at 11:11:24AM +0200, Kostya wrote:
> Well, it roughly the same feature-wise as it was,
> I just don't think I will improve it soon, yet
> it is playable (and maybe will attract samples
> and patches, I'm an optimist).

last part: rv40.c

> +static int rv40_parse_slice_header(RV34DecContext *r, GetBitContext *gb, SliceInfo *si)
> +{
> +    int t, mb_bits;
> +    int w = r->s.width, h = r->s.height;
> +    int mb_size;
> +
> +    memset(si, 0, sizeof(SliceInfo));
> +    if(get_bits1(gb))
> +        return -1;
> +    si->type = get_bits(gb, 2);
> +    if(si->type == 1) si->type = 0;
> +    si->quant = get_bits(gb, 5);
> +    if(get_bits(gb, 2))
> +        return -1;
> +    si->vlc_set = get_bits(gb, 2);

> +    get_bits1(gb);

skip_bits1();

> +    t = get_bits(gb, 13); /// ???
> +    if(!si->type || !get_bits1(gb))
(Continue reading)

Permalink | Reply | 0 comments |
headers
Michael Niedermayer | 1 Dec 2007 02:42
Picon
Picon

Re: [FFmpeg-cvslog] r11100 - in trunk/libavcodec/i386: cavsdsp_mmx.c dsputil_mmx.c dsputil_mmx.h h264dsp_mmx.c mpegvideo_mmx.c vc1dsp_mmx.c

On Sat, Dec 01, 2007 at 02:00:57AM +0200, Uoti Urpala wrote:
> On Fri, 2007-11-30 at 21:11 +0100, Michael Niedermayer wrote:
> > On Fri, Nov 30, 2007 at 07:04:42PM +0200, Uoti Urpala wrote:
> > > Any linker stuff like (3) or (4) cannot give the full available
> > speedup;
> > 
> > yes, but adding -Bsymbolic to ld is easy, getting the vissibility correct
> > migh not be, i dunno, iam happy with a clean patch which fixes the vissibility
> > of symbols to the minimum overrideable and vissible as well
> > still i think (3) is simpler ...
> 
> Getting visibility correct is very straightforward and obvious if you do
> it per symbol, but requires a code change for each symbol whose
> visibility you want to change. Basically you'd have 'static' for things
> visible only in the same translation unit, and 'FF_PRIVATE' (or some
> better macro name) for things visible only in the same library. 

i know that, the problems are
1. the amount of symbols
2. its often not completely clear if something should or should not be
   vissible from the outside

> Pragmas
> allow getting the result with much fewer code changes but result in less
> obvious code.
> 

> -Bsymbolic is "simpler" of course but it does not achieve the same
> things.
(Continue reading)

Permalink | Reply | 1 comment |
headers
Uoti Urpala | 1 Dec 2007 03:06
Picon
Picon

Re: [FFmpeg-cvslog] r11100 - in trunk/libavcodec/i386: cavsdsp_mmx.c dsputil_mmx.c dsputil_mmx.h h264dsp_mmx.c mpegvideo_mmx.c vc1dsp_mmx.c

On Sat, 2007-12-01 at 02:42 +0100, Michael Niedermayer wrote:
> On Sat, Dec 01, 2007 at 02:00:57AM +0200, Uoti Urpala wrote:
> > -Bsymbolic is "simpler" of course but it does not achieve the same
> > things.
> 
> it fixes the link failure which is the orignal issue and that has to be
> fixed, waiting several month for any per symbol work just is no alternative
> no matter if or if not its a good idea on its own

There's no need to do it for all symbols at once. If you think it's a
good idea in principle then all that's needed is a definition of the
macro somewhere (defined to __attribute__((visibility("hidden"))) if
supported and to empty otherwise) and then you can start adding it to
individual symbols, starting from the ones now causing compilation
failure on AMD64. That could be done within hours.
Permalink | Reply | 0 comments |
headers
Michael Niedermayer | 1 Dec 2007 03:46
Picon
Picon

Re: Merging the fixed point WMA decoder

On Wed, Nov 28, 2007 at 01:56:43PM -0500, Michael Giacomelli wrote:
> I'd like to start working on merging the fixed point wma decoder back into
> ffmpeg as soon as my other projects permit.  I have some questions about how
> to do this:
> 
> 1)  We don't really have much fixed point transform or dsputil code in
> place.  I've written some that works fine for this particular codec, but
> inserting it into ffmpeg is unclear to me.  Is it ok to make a
> wmaprecision.h file and import it into dsputil.c and elsewhere so that I can

no, a #include "wmaprecision.h" in dsputil.c/h is not ok

> #define the correct function calls there for things like
> vector_fmul_reverse, CMUL, etc?  Or should the wma codec somehow overwrite
> those function pointers and defines if needed during it's init?
> 

> 2)  Although most of the codec is pretty easy to make both fixed and fp just
> by playing with type defs and redefining what mul(), CMUL, etc does, theres
> some parts where it is not.  Is it acceptable to define function_fixed and
> function_float for these cases?

yes

>  Also, it is frequently necessary to rescale
> fixed point values in the middle of functions.  Is it acceptable to #define
> blocks in and out in the middle of a function?

no unless it absolutely needed
also the amount of rescalng should be kept to a minimum
(Continue reading)

Permalink | Reply | 0 comments |
headers
Uoti Urpala | 1 Dec 2007 03:53
Picon
Picon

Re: [FFmpeg-cvslog] r11100 - in trunk/libavcodec/i386: cavsdsp_mmx.c dsputil_mmx.c dsputil_mmx.h h264dsp_mmx.c mpegvideo_mmx.c vc1dsp_mmx.c

On Fri, 2007-11-30 at 17:56 -0700, Loren Merritt wrote:
> On Fri, 30 Nov 2007, Uoti Urpala wrote:
> 
> > Would (3) or (4) actually fix compilation in this case? I think the
> > addressing mode generated by the compiler causes the problems and AFAIK
> > the linker wouldn't change that.
> 
> But this addressing is not generated by the compiler, hence the problem.
> 
> Static library: inline asm contains a textrel. ok.
> Shared library: inline asm still contains a textrel. linker barfs because 
> that's not PIC.
> Shared library with -Bsymbolic: inline asm still contains a textrel. 
> linker resolves the textrel when making the shared library, thus there 
> are no textrels in the .so, so it's ok.

Yes in this case the generated addressing mode was apparently
PC-relative so the relocation could be removed by the linker (set to the
constant offset to the variable in the same library).

> > Any linker stuff like (3) or (4) cannot give the full available speedup;
> > for that the addressing mode needs to be known at compile time. As I
> > wrote in another message the visibility should be marked in the headers
> > with either a pragma changing default visibility or a per-symbol macro.
> > Those produce identical results but differ in code maintenance
> > qualities.
> 
> Benchmarks (of x264 not ffmpeg, but they should still be relevant):
> 
> x86_64 (Core2)
(Continue reading)

Permalink | Reply | 8 comments |
headers
Michael Niedermayer | 1 Dec 2007 03:57
Picon
Picon

Re: [PATCH] Make libavformat/url_open print an error message in case of unrecognized protocol string

On Thu, Nov 29, 2007 at 10:07:55AM +0100, Stefano Sabatini wrote:
[...]

> By the way, we should somehow fix the avcodec.h AVERROR_ mess.

yes ...

[...]
> Index: libavformat/avio.c
> ===================================================================
> --- libavformat/avio.c	(revision 11111)
> +++ libavformat/avio.c	(working copy)
>  <at>  <at>  -68,6 +68,7  <at>  <at> 
>              goto found;
>          up = up->next;
>      }
> +    av_log(NULL, AV_LOG_ERROR, "Unrecognized protocol string: %s\n", proto_str);
>      err = AVERROR(ENOENT);
>      goto fail;
>   found:

i dont like printing an error message in just one of several failure cases
this is inconsistant
and printing one in all is also not really nice

anyway its a very minor issue, i wont accept ugly hacks to fix it ...

[...]
--

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
(Continue reading)

Permalink | Reply | 0 comments |

Gmane