[prev in list] [next in list] [prev in thread] [next in thread]
List: helix-datatype-dev
Subject: Re: [datatype-dev] CR-Client: datatype project
From: Jamie Gordon <jgordon () real ! com>
Date: 2007-04-27 13:15:46
Message-ID: 4631F782.7020700 () real ! com
[Download RAW message or body]
Wang JinJian-E12714 wrote:
> Thanks,
> So how can we handle this case?
You will need to debug to verify whether the problem is a bad ID3 tag
length field causing the parsing to begin at the wrong location, or a
similar issue with the file. If so and this type of erroneous file does
need to be handled, then likely the proper solution is to update the
handling as mentioned to not perform the checks for other file types
in case of parsing after an ID3v2 tag.
Thanks,
Jamie
>
> -----Original Message-----
> From: jgordon@real.com [mailto:jgordon@real.com]
> Sent: Wednesday, April 25, 2007 6:24 AM
> To: Wang JinJian-E12714
> Cc: Jamie Gordon; Lu Mingjun-E7978C; datatype-dev@helixcommunity.org; Qu
> Yongzhi-W20664; Zhao Liang-E3423C; Xing Hongjiang-e7263c
> Subject: RE: [datatype-dev] CR-Client: datatype project
>
>> Yes, I know the code section is for MPEG-1 or MPEG-2 container! But
>> our fix didn't breakup any logic of the normal case of it! Only the
>> error case will be resync to a correct mp3 syncword!
>>
>
> So the problem is that, in order to support mpeg audio (mp3/mpa/etc)
> files with extensions that overlap with mpeg 1/2 video/container files
> (.mpg extension), this code is in place to handle the case where an MPEG
> audio frame was not found where expected at the start, but rather an
> MPEG start code - in that case it needs to pass the HXR_NEED_UPGRADE to
> alert the core that this is not a valid MP3 file and that if no other
> handler is found, it might query to download, etc. based on extension.
> This is ugly, but I believe still necessary!
>
> This change looks will change all such cases to ignore that a start code
> was found and instead search for anything that looks like a valid MPEG
> audio frame. This is not right. Even if handling MPEG start codes to
> signal MPEG container rather than MP3 is no longer necessary, then this
> still doesn't handle the other cases, only the case where a container
> file is thought to be found.
>
>
>> This case is not a ID3 issue because all of the ID3 parser is correct!
>> This is a special case which contain some unknow data after ID3, and
>> these data is not ZERO data or valid MP3/MPEG-1/MPEG-2 start code!
>> That's why original code will report a error of NEED UPGRADE!
>>
> The problem is that after the ID3 tag, when the ff plugin is looking for
> MP3 data, it is not finding it where it is expected, but then it is
> finding something that looks like an MPEG start code. From a cursory
> look, this appears to be a problem with the file - it looks like the
> 'size' field in the ID3 tag is set to '0'. It might also be a problem
> with ID3 parsing, etc. that is looking to the wrong place.
>
> Assuming it is not simply a bug to fix in ID3 handling, there are a
> couple of possibilities here if files like this need to be supported.
> First thing, what are the searches and the order they are done in? Need
> to make sure that makes sense - I'm not certain, but it looks like it
> might do an initial byte compare for audio then a search for start
> codes, which does not seem correct. So if this is the case, this is
> likely a good place to look for correction.
>
> Secondly, if an ID3v2 tag is present, I believe it is safe to assume (in
> that case only) that the content is MPEG audio and not MPEG 1/2
> container/video or any of the other file types that are being checked
> for there, so all the special handling of various other file types can
> probably be skipped when searching after an ID3 tag.
>
> Thanks,
> Jamie
>
>> Attached is the MP3 file and you can get more inform from it! And we
>> wish get a right fix of it from you!
>>
>> BRs
>> -Wang Jinjian
>>
>> -----Original Message-----
>> From: Jamie Gordon [mailto:jgordon@real.com]
>> Sent: Friday, April 06, 2007 4:36 AM
>> To: Lu Mingjun-E7978C
>> Cc: datatype-dev@helixcommunity.org; Wang JinJian-E12714; Qu
>> Yongzhi-W20664; Zhao Liang-E3423C; Xing Hongjiang-e7263c
>> Subject: Re: [datatype-dev] CR-Client: datatype project
>>
>> Please be more descriptive with your CR subjects. "datatype project"
>> is not descriptive enough - it should at the very least mention that
>> this is a change to the mp3 file format - something very much like the
>
>> "Synopsis" you've got would be appropriate.
>>
>> This change is not the correct fix. The code you are changing is the
>> handling of the specific case where the file is recognized as an
>> MPEG-1 or MPEG-2 container or video file rather than an MP3/MPA file
>> and is passed off to the appropriate plugin (or updater, etc.).
>>
>> The problem you are describing sounds like either a problem with ID3
>> length parsing or a bad file. If there is a problem with the ID3
>> parsing, then that problem must be fixed. If the problem is a bad
>> file, then if this particular problem must be handled then the
>> work-around must be written in a way that does not break the handling
>> of MPEG files, etc.
>>
>> Thanks,
>> Jamie
>>
>> Lu Mingjun-E7978C wrote:
>>> Modified by: e12714@motorola.com <mailto:e12714@motorola.com>
>>>
>>> Date: 04:04:2007
>>>
>>> Project: mediaplayer on motorola phone Bug Number: Bug URL:
>>>
>>> Synopsis: mp3 with unkonw data after ID3 tag can not be played
>>>
>>> Overview: <detailed description>
>>>
>>> in some mp3 file, after ID3, there is 295 bytes
>>> unknow_data,ID3_tag(4332) + unknow_data(295) + mp3_valid_data. But
>>> seems helix doesn't pass through these unknow_data.
>>>
>>> Files Added: [File 1] - <purpose synopsis> [File 2] - <purpose
>>> synopsis>
>>>
>>>
>>> Files Modified: [File 1] - datatype/mp3/fileformat/mp3ff.cpp
>>>
>>>
>>> Image Size and Heap Use impact (Client -Only): <Description of image
>>> size and heaps size impact. >
>>>
>>> Platforms and Profiles Affected: <List of profiles & platforms
>>> affected and comments on how they are affected if they are different>
>>>
>>>
>>> Distribution Libraries Affected:
>>>
>>> Distribution library impact and planned action: <Is an update
>>> required
>>> and if so when will it occur>
>>>
>>> Platforms and Profiles Build Verified: build for arm11 version
>>>
>>> Platforms and Profiles Functionality verified: <List of profiles &
>>> platforms for which the functionality was verified>
>>>
>>> Branch: <code branch(es) change is for>
>>>
>>> Copyright assignment: <3. >
>>>
>>> 1. In consideration for RealNetworks' hosting and maintenance of
>>> my modification, I agree to assign to RealNetworks full copyright
>>> ownership of the code included in the attached patch, and agree that
>>> RealNetworks has no duty of accounting to me for it. I warrant that
>>> this code is entirely original to and owned by me, that I can legally
>
>>> grant the copyright assignment, and that my contribution does not
>>> violate any other person's rights, and laws or breach any contract. I
>
>>> understand that RealNetworks may license this code under RPSL, RCSL,
>>> and/or any other license at RealNetworks' discretion, and use the
>>> code
>>> in any way.
>>>
>>> 2. I have signed and delivered a Joint Copyright Assignment to
>>> RealNetworks, and received acknowledgment that the agreement was
>>> received.
>>>
>>> 3. My company (Motorola) submits this code under the terms of a
>>> commercial contribution agreement with RealNetworks, and I am
>>> authorized to contribute this code under said agreement.
>>>
>>> 4. I am a RealNetworks employee or contractor
>>>
>>> QA Instructions: <Description of functionality affected & suggestions
>
>>> on verification areas>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> -
>>> --
>>>
>>>
>>> _______________________________________________ Datatype-dev mailing
>>> list Datatype-dev@helixcommunity.org
>>> http://lists.helixcommunity.org/mailman/listinfo/datatype-dev
>
_______________________________________________
Datatype-dev mailing list
Datatype-dev@helixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/datatype-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic