[Ffmpeg-devel] THP decoder

Kislyakov Maxim spidy
Sat Apr 7 17:26:54 CEST 2007


> Hi
Hi

Michael Niedermayer wrote:
> On Sat, Apr 07, 2007 at 05:55:08PM +0400, Kislyakov Maxim wrote:
>> > Hello,
>> 
>> Hi
>> 
>> Reimar Doeffinger wrote:
>> > On Sat, Apr 07, 2007 at 05:27:54PM +0400, Kislyakov Maxim wrote:
>> >> Michael Niedermayer wrote:
>> >> > no, they are doxygen comments now but they are still wrong
>> >> 
>> >> I hope now it is ok, may be i should remove all comments? =)
>> >> if (no)
>> >>     HOW_TO_WRITE_THEM;
>> >> else
>> >>     THANKS;
>> > 
>> > ///-comments AFAIK apply to the next line, but these are for the thing
>> > to the left. Checking other files like avformat.h indicates that
>> > ///<  is the right style.
>> 
>> fixed
>> New patch attached (9th this time)
>> 
> [...]
> 
>> +    get_be32(pb);                                 ///< Skipping magic
>> bytes
> 
> doxygen doesnt support comments placed like that

fixed

>> +    version = get_le32(pb);
>> +    if (version != 0x100 && version != 0x110)
>> +        return AVERROR_IO;                        ///< Incorrect version
> 
> what is this check good for?

removed

> [...]
>> +    if (thpDemux->componentTypes[0] == 0) {
>> +        st->codec->width = get_be32(pb);
>> +        st->codec->height = get_be32(pb);
>> +        if (version == 0x110)
>> +            get_be32(pb);
>> +    }
> 
> what if video is not the first component?

Video is the first component always.

> [...]
>> +    thpDemux->curFrame = thpDemux->firstFrameOffset;
>> +    thpDemux->frameSize = thpDemux->firstFrameSize;
> 
> what are the first* variables good for? why not read in the correct ones
> directly?

fixed

> [...]
>> +        pkt->stream_index = 1;
> 
> this is wrong if there is no video (i dont know if that can occure but your
> demuxer is checking for the case so i assume it can)

Video is always there because it's video format, audio may be
optional (it's written in specification)

> [...]
>> +static inline short adpcm_thp_expand_nibble(ADPCMThpFrameHeader *fh,
>> int8_t nibble,
>> +                                            uint8_t index, uint8_t
>> exponent, int st)
>> +{
>> +    short temp = 0;
>> +    int factor1, factor2;
>> +    if (nibble & 0x08)  {
>> +        nibble = nibble | 0xf0;
> 
> |=
> 
>> +    }
> 
> superfluous {}

removed

This is new patch.

Maxim
-------------- next part --------------
A non-text attachment was scrubbed...
Name: thp-patchvA.diff
Type: application/octet-stream
Size: 10371 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070407/b69fa509/attachment.obj>



More information about the ffmpeg-devel mailing list