Re: [Android-port-dev] CR: extend CHXFileRecognizer to cover more mine-types
Qiang Luo <[email protected]>
| Newsgroups | gmane.comp.multimedia.helix.devel |
|---|---|
| Message-ID | <[email protected]> |
Sheldon,
I took out the extension checking code. Please review attached new diff.
Here are a list of changes from the last CR diff:
1) skiped format checking if there is a file extension,
2) added mp3 frame header validation,
4) added AVI format detection,
5) fixed rm detection to include rms and ra magic,
6) re-ordered the detection sequence to check for media container
magic first, avoiding unnecessary data scan.
7) added comment for #if defined(ANDROID)
8) fixed typo (thanks Adam)
The detection list is not complete. Please review what I have.
Thanks,
Qiang
At 12:47 PM 2/19/2009, Sheldon Fu wrote:
>That's the other thing. The code is as generic as it could be. Why
>wrapped it around "#ifdef ANDROID"? You'll have to put some comment for that.
>
>If the reason is to minimize the impact on other platforms since we
>don't have time to test and verify, as long as you maintain the hope
>that one day the "#ifdef ANDROID" will be taken out, you'll have to
>design for that generic use.
>
>I would say we don't need this list. Being able to play a renamed
>.bin file is nice but not the purpose of this Android porting task.
>It can wait until some other time.
>
>fxd
>
>Qiang Luo wrote:
>>Sheldon,
>>
>>Your arguments sound convening but the assumption that for new mp5,
>>"your mp4 detection code will be fooled to think it is mp4" is not
>>fair. This code is for Android platform only. When you decide to
>>support 'mp5' on Android, you will have to add 'mp5' detection code
>>to the FileRecognizer and fix any other detection code to not to be
>>"fooled" by 'mp5'. Remember, for Android, we need to support the
>>format detection in FileRecognizer to accommodate file descriptor
>>as data source.
>>
>>Having said that, I'm willing to take out the extension checking
>>code if there are still objections.
>>
>>Thanks,
>>
>>Qiang
>>
>>At 10:53 AM 2/19/2009, Sheldon Fu wrote:
>>>Just because it doesn't break anything, YET, doesn't mean it's
>>>correct. E.g, assuming there is a promising new file format called
>>>'mp5', that we decide to have support for in the future, so we
>>>write a new file format plugin for it. And assume that it is
>>>similar to mp4 enough so your mp4 detection code will be fooled to
>>>think it is mp4. Since 'mp5' is not on the hard-coded list you
>>>have, your code will fail it, though we do have a file format for it.
>>>
>>>While coming back to add 'mp5' to the list will fix the issue,
>>>nobody is going to remember to do that. I hope you can see the
>>>problem with a hard-coded/arbitrary list of extensions.
>>>
>>>fxd
>>>
>>>Qiang Luo wrote:
>>>>At 09:33 AM 2/19/2009, Sheldon Fu wrote:
>>>>>1. It's not about what you do with file with extension not in
>>>>>the list. It's about why you skip the file that has extension on
>>>>>the list. Taking '.bin' in your example, how do you know that
>>>>>there is not a file format plugin out there that claims 'bin'
>>>>>already or there won't be one in the future? Another way to
>>>>>think about this is to answer the question "how did you come up with the list?"
>>>>
>>>>I do not see any logical problem in my design. If you disagree,
>>>>you need to construct an example to show that a track would play
>>>>fine with the original code but would fail with my changes. As I
>>>>explained in my last response, having the extension list gives us
>>>>the ability to check with recognizer and play contents even if
>>>>the user renames the content file extension. Any issue followed
>>>>by having this feature is then purely performance tread off.
>>>>
>>>>Qiang
>>>>
>>>>>2. This is another reason why a detection logic in TLC may be
>>>>>more appropriate. The logic here is never designed to be
>>>>>powerful enough since it is only a fall-back in core but on
>>>>>Android, we need this detection logic to be the primary way of
>>>>>recognizing and playing back local content.
>>>>>
>>>>>3. Again it would be useful to study the media player comes with
>>>>>Android source and make sure that whatever it can recognize and play, we can.
>>>>>
>>>>>fxd
>>>>>
>>>>>
>>>>>Qiang Luo wrote:
>>>>>>Thanks for the comments Sheldon. My responses inline.
>>>>>>
>>>>>>At 05:12 AM 2/19/2009, Sheldon Fu wrote:
>>>>>>>Couple of comments,
>>>>>>>
>>>>>>>1. The concept of hard-coded 'known extension' in client core
>>>>>>>is not very appropriate. Helix DNA can be extended by adding
>>>>>>>new file format plugins, which will claim new extensions they
>>>>>>>support. Also, build config has choices of which file format
>>>>>>>plugin to include in the final installation. There is really
>>>>>>>not a list of 'known' extensions to the core, unless you
>>>>>>>create this list by dynamically query each file format plugin
>>>>>>>for extensions they support. The best we can do here is to
>>>>>>>skip the detection if there is any extension I think.
>>>>>>
>>>>>>The purpose of having this list is to trigger the recognizer's
>>>>>>detection process if the given extension is not on the
>>>>>>list. This allows us to play something like a_m4a_track.bin
>>>>>>instead of just giving it up. If an extension at hand is not
>>>>>>on the list but the player has the ff plugin to handle it, then
>>>>>>we pay the performance penalty of doing the detection. In
>>>>>>general, this is a feature-performance trade off issue. For
>>>>>>Android(the change is #if defined() for android), there
>>>>>>shouldn't be any problem. We will have a collection of media
>>>>>>formats that we must handle, therefor a list of corresponding
>>>>>>extensions to skip detection.
>>>>>>
>>>>>>I will rename function "IsKnownMediaExtension(const char*
>>>>>>pFileExtension)" to "ShouldSkipFormatDetection(const char* pFileExtension)".
>>>>>>
>>>>>>>2. ID3 isn't always at the front of MP3 file. It could be at
>>>>>>>the end and it might not present at all. I am not familiar
>>>>>>>with other data types, need to make sure we don't artificially
>>>>>>>restrict what we can play here.
>>>>>>
>>>>>>The format detection code for mp3(and others as well) needs to
>>>>>>be improved. In addition to the case you raised above, not all
>>>>>>files with ID3 tags are MP3s. I will add code to locate and
>>>>>>validate the first frame header. Reliable detection may not be
>>>>>>always possible given the limited amount of data(512 byte) at
>>>>>>the start of the file.
>>>>>>
>>>>>>
>>>>>>>3. What about OGG and WAV?
>>>>>>>
>>>>>>>Since this is the primary code path for playing a local file
>>>>>>>on Android, we need to make sure whatever we want to be able
>>>>>>>to play will come out of this code with an appropriate
>>>>>>>mime-type. We should have a list of data-types we claim we
>>>>>>>support on Android and we need to make sure all of them are taken care of here.
>>>>>>>
>>>>>>>And because we are hoping to be an alternative media engine
>>>>>>>solution for Android, we better make sure whatever the current
>>>>>>>media engine can play, we can too. E.g, if we only play MP3s
>>>>>>>with ID3 tag at the front but current media engine plays when
>>>>>>>ID3 tag is at the end or without ID3 tag at all, it won't be a
>>>>>>>convincing story.
>>>>>>>
>>>>>>>fxd
>>>>>>
>>>>>>Tony, could you send me a list of file mime-types that we must
>>>>>>support in Android?
>>>>>>
>>>>>>Thanks,
>>>>>>
>>>>>>Qiang
>>>>>>
>>>>>>>Qiang Luo wrote:
>>>>>>>>Synopsis:
>>>>>>>>Extend CHXFileRecognizer to cover additional mine-types for
>>>>>>>>Android project.
>>>>>>>>
>>>>>>>>Summary:
>>>>>>>>In Android project, we are introducing a new protocol scheme,
>>>>>>>>fd://, claimed by the local file system, to support file
>>>>>>>>descriptor as data source to the helix player. Normally, for
>>>>>>>>the local file source, the the client core uses the file
>>>>>>>>extension to load the file format plugin via IHXPluginHandler3.
>>>>>>>>When using descriptor as data source, the content file name
>>>>>>>>and extension will be unknown. Therefore, we need to extend
>>>>>>>>the SDP-only CHXFileRecognizer to cover additional mine-types
>>>>>>>>for Android project.
>>>>>>>>
>>>>>>>>The client core HXFileSource setup code has the following
>>>>>>>>mine-type discovery order:
>>>>>>>>
>>>>>>>>1) look for "Content-Type" property in the request's response header.
>>>>>>>>2) QI the file-system's file object for IHXFileMimeMapper
>>>>>>>>interface and call FindMimeType().
>>>>>>>>3) get mime-type from url parameters.
>>>>>>>>4) try the file recognizer GetMimeType(m_pFileObject,
>>>>>>>>m_pMimeFinderResponse).
>>>>>>>>5) try to create a file format plugin enumerator via
>>>>>>>>IHXPluginHandler3 based on the file extension to load the plugin
>>>>>>>>6) fail if none of the above attempts succeeds.
>>>>>>>>
>>>>>>>>Adding additional mine-types detection to CHXFileRecognizer
>>>>>>>>would alter the client core code path. To minimize the risk
>>>>>>>>to other projects, I add #if defined() to make the addition
>>>>>>>>Android platform only. Even for Android, we defer mime-type
>>>>>>>>detection to FileRecognizer only when there is no file-name,
>>>>>>>>or the file extension is not a known media type.
>>>>>>>>
>>>>>>>>Note, the detection code must be simple and small in
>>>>>>>>size. It is intended to be just robust enough to load the
>>>>>>>>correct file format plugin.
>>>>>>>>
>>>>>>>>Branch:
>>>>>>>>head, hxclient_3_1_0_atlas
>>>>>>>>
>>>>>>>>File modified:
>>>>>>>>common/system/recognizer.cpp
>>>>>>>>common/system/pub/recognizer.h
>>>>>>>>
>>>>>>>>Affected:
>>>>>>>>client core.
>>>>>>>>
>>>>>>>>Risks:
>>>>>>>>high.
>>>>>>>>
>>>>>>>>Please review the attached diff. I'm looking forward to
>>>>>>>>suggestions to improve the format detection code.
>>>>>>>>
>>>>>>>>Thanks,
>>>>>>>>
>>>>>>>>Qiang
Index: recognizer.cpp
===================================================================
RCS file: /cvsroot/common/system/recognizer.cpp,v
retrieving revision 1.12
diff -u -w -1 -0 -r1.12 recognizer.cpp
--- recognizer.cpp 7 Feb 2006 19:21:27 -0000 1.12
+++ recognizer.cpp 20 Feb 2009 23:58:12 -0000
@@ -1,53 +1,304 @@
#include "ihxpckts.h"
#include "hxbuffer.h"
#include "pckunpck.h"
#include "recognizer.h"
+// mime-types our recognizer might return
+#define HX_MIMETYPE_RM "application/x-pn-realmedia"
+#define HX_MIMETYPE_MP3 "audio/mp3"
+#define HX_MIMETYPE_MP4AUDIO "audio/mp4"
+#define HX_MIMETYPE_MP4VIDEO "video/mp4"
+#define HX_MIMETYPE_3GPAUDIO "audio/3gpp"
+#define HX_MIMETYPE_3GPVIDEO "video/3gpp"
+#define HX_MIMETYPE_WM "application/vnd.ms-asf"
+#define HX_MIMETYPE_AVI "application/x-pn-avi-plugin"
+#define HX_MIMETYPE_SDP "application/sdp"
+
+// length of buffer read from file and passed to recognizer
+static const int KRecogLength = 512;
+
#if !defined (_SYMBIAN)
HX_RESULT CHXFileRecognizer::GetMimeType(const char* pFileName,
IHXBuffer* pBuffer, REF(IHXBuffer*) pMimeType)
{
- if (IsSDPFile(pBuffer))
+ HX_RESULT retVal = HXR_FAILED;
+ char* pMimeTypeString = NULL;
+ char* pFileExtension = NULL;
+ UCHAR* pData = NULL;
+ UINT32 ulSize = 0;
+ HXBOOL bContinue = TRUE;
+
+ // get file extension
+ if (pFileName)
{
- if (HXR_OK == CreateBufferCCF(pMimeType, m_pContext))
+ CHXString strExtension = pFileName;
+ INT32 lPeriod = strExtension.ReverseFind('.');
+ if (lPeriod >= 0)
+ {
+ strExtension =
strExtension.Right(strExtension.GetLength() - lPeriod - 1);
+ if (strExtension.GetLength() > 0)
+ {
+ strExtension.MakeLower();
+ pFileExtension = (char*)(const char*)strExtension;
+ }
+ }
+ }
+
+ // data at the begining of the media file
+ if (pBuffer)
+ {
+ ulSize = pBuffer->GetSize();
+ pData = pBuffer->GetBuffer();
+ }
+
+ // check SDP
+ if (IsSDPFile(pData, ulSize))
+ {
+ pMimeTypeString = HX_MIMETYPE_SDP;
+ bContinue = FALSE;
+ }
+
+ //XXXqluo to minimize the risk, the following format check is
for ANDROID platform only
+#if defined(ANDROID)
+ // skip checking if the file name has an extension.
+ if (bContinue && pFileExtension || ulSize != KRecogLength)
{
- int len = strlen("application/sdp");
- pMimeType->Set((const UCHAR*)"application/sdp", len + 1);
+ bContinue = FALSE;
+ }
+
+ // RealMedia
+ if (bContinue && IsRMFile(pData, ulSize))
+ {
+ pMimeTypeString = HX_MIMETYPE_RM;
+ bContinue = FALSE;
+ }
+
+ // 3GPP
+ if (bContinue && Is3GPPFile(pData, ulSize))
+ {
+ pMimeTypeString = HX_MIMETYPE_3GPAUDIO;
+ bContinue = FALSE;
+ }
+
+ // WM
+ if (bContinue && IsWMFile(pData, ulSize))
+ {
+ pMimeTypeString = HX_MIMETYPE_WM;
+ bContinue = FALSE;
+ }
+
+ // AVI
+ if (bContinue && IsAVIFile(pData, ulSize))
+ {
+ pMimeTypeString = HX_MIMETYPE_AVI;
+ bContinue = FALSE;
+ }
+
+ // MP4
+ HXBOOL bVideo = FALSE;
+ if (bContinue && IsMP4File(pData, ulSize))
+ {
+ pMimeTypeString = HX_MIMETYPE_MP4AUDIO;
+ bContinue = FALSE;
+ }
+
+ // MP3
+ if (bContinue && IsMP3File(pData, ulSize))
+ {
+ pMimeTypeString = HX_MIMETYPE_MP3;
+ bContinue = FALSE;
+ }
+#endif // ANDROID
+
+ if (pMimeTypeString)
+ {
+ retVal = CreateBufferCCF(pMimeType, m_pContext);
+ if (HXR_OK == retVal)
+ {
+ int len = strlen(pMimeTypeString);
+ pMimeType->Set((const UCHAR*)pMimeTypeString, len + 1);
((char*)pMimeType->GetBuffer())[len] = '\0';
- return HXR_OK;
}
}
- return HXR_FAILED;
+ return retVal;
}
#endif
HXBOOL
-CHXFileRecognizer::IsSDPFile(IHXBuffer* pBuffer)
+CHXFileRecognizer::IsSDPFile(UCHAR* pData, UINT32 ulSize)
{
HXBOOL bResult = FALSE;
+ if (pData && ulSize > 2)
+ {
+ if (0 == strncmp((const char*)pData, "v=", 2))
+ {
+ bResult = TRUE;
+ }
+ }
+ return bResult;
+}
- if (pBuffer && pBuffer->GetSize())
+HXBOOL
+CHXFileRecognizer::IsRMFile(UCHAR* pData, UINT32 ulSize)
+{
+ HXBOOL bResult = FALSE;
+ UCHAR pRAMagic[4] = {'.', 'r', 'a', 0xFD};
+ if (pData && ulSize > 4)
{
- if (0 == strncmp((const char*)pBuffer->GetBuffer(), "v=", 2))
+ if (0 == strncmp((const char*)pData, ".RMF", 4) ||
+ 0 == strncmp((const char*)pData, ".RMS", 4) ||
+ 0 == memcmp(pData, pRAMagic, 4))
{
bResult = TRUE;
}
}
+ return bResult;
+}
+
+HXBOOL
+CHXFileRecognizer::IsMP4File(UCHAR* pData, UINT32 ulSize)
+{
+ if (IsISOMediaFile(pData, ulSize, "M4A") ||
+ IsISOMediaFile(pData, ulSize, "mp4"))
+ {
+ return TRUE;
+ }
+ else
+ {
+ return FALSE;
+ }
+}
+
+HXBOOL
+CHXFileRecognizer::Is3GPPFile(UCHAR* pData, UINT32 ulSize)
+{
+ return IsISOMediaFile(pData, ulSize, "3gp");
+}
+HXBOOL
+CHXFileRecognizer::IsWMFile(UCHAR* pData, UINT32 ulSize)
+{
+ HXBOOL bResult = FALSE;
+ if (pData && ulSize > 16)
+ {
+ UCHAR pMagic[16] = {0x30, 0x26, 0xB2, 0x75, 0x8E, 0x66, 0xCF,
0x11, 0xA6, 0xD9, 0x00, 0xAA, 0x00, 0x62, 0xCE, 0x6C};
+ if (memcmp(pData, pMagic, 16) == 0)
+ {
+ bResult = TRUE;
+ }
+ }
return bResult;
}
-// length of buffer read from file and passed to recognizer
-static const int KRecogLength = 512;
+HXBOOL
+CHXFileRecognizer::IsAVIFile(UCHAR* pData, UINT32 ulSize)
+{
+ HXBOOL bResult = FALSE;
+ if (pData && ulSize > 16)
+ {
+ UCHAR pMagic1[4] = {'R', 'I', 'F', 'F'}; // RIFF
+ UCHAR pMagic2[8] = {'A', 'V', 'I', ' ', 'L', 'I', 'S',
'T'}; // AVI LIST chunk
+ if (memcmp(pData, pMagic1, 4) == 0 ||
+ memcmp(pData+8, pMagic2, 8) == 0)
+ {
+ bResult = TRUE;
+ }
+ }
+ return bResult;
+}
+
+HXBOOL
+CHXFileRecognizer::IsMP3File(UCHAR* pData, UINT32 ulSize)
+{
+ HXBOOL bResult = FALSE;
+ if (pData && ulSize)
+ {
+ // check for ID3 tags
+ if (0 == strncmp((const char*)pData, "ID3", 3))
+ {
+ bResult = TRUE;
+ }
+ else
+ {
+ // check for valid frame header
+ UINT32 ulHdr;
+ UCHAR* pEnd = pData + ulSize - 4;
+ do
+ {
+ ulHdr = LoadMP3Header(pData);
+ if (IsValidMP3Header(ulHdr))
+ {
+ bResult = TRUE;
+ break;
+ }
+ pData += 4;
+ }
+ while(pData < pEnd);
+ }
+ }
+ return bResult;
+}
+
+UINT32
+CHXFileRecognizer::LoadMP3Header(UCHAR* p)
+{
+ return (UINT32)(((p[0] & 255) << 24) | ((p[1] & 255) << 16) |
((p[2] & 255) << 8) | ((p[3] & 255)));
+}
+
+HXBOOL
+CHXFileRecognizer::IsValidMP3Header(UINT32 ulHdr)
+{
+ return (((getFrameSync(ulHdr) & 2047)==2047) &&
+ ((getVersionIndex(ulHdr) & 3)!= 1) &&
+ ((getLayerIndex(ulHdr) & 3)!= 0) &&
+ ((getBitrateIndex(ulHdr) & 15)!= 0) &&
+ ((getBitrateIndex(ulHdr) & 15)!= 15) &&
+ ((getFrequencyIndex(ulHdr) & 3)!= 3) &&
+ ((getEmphasisIndex(ulHdr) & 3)!= 2) );
+}
+
+HXBOOL
+CHXFileRecognizer::IsISOMediaFile(UCHAR* pData, UINT32 ulSize, const
char* pBrand)
+{
+ HXBOOL bResult = FALSE;
+ if (pData && ulSize)
+ {
+ UCHAR* pEnd = (pData + ulSize - 4);
+
+ // locate file type box
+ pData += 4;
+ HXBOOL bFound = FALSE;
+ while (pData < pEnd)
+ {
+ // scan for "ftyp"
+ if ((*pData == 'f' && *(pData+1) == 't' && *(pData+2) ==
'y' && *(pData+3) == 'p'))
+ {
+ bFound = TRUE;
+ break;
+ }
+ pData++;
+ }
+ // match for the given type(pBrand)
+ if (bFound)
+ {
+ pData += 4;
+ if (0 == strncmp((const char*)pData, pBrand, strlen(pBrand)))
+ {
+ bResult = TRUE;
+ }
+ }
+ }
+ return bResult;
+}
CHXFileRecognizer::CHXFileRecognizer(IUnknown* pContext)
: m_lRefCount(0),
m_pResponse(NULL),
m_pFile(NULL),
m_pBuffer(NULL),
m_bGetMimeTypeDone(FALSE),
m_pContext(pContext)
{
HX_ADDREF(m_pContext);
Index: pub/recognizer.h
===================================================================
RCS file: /cvsroot/common/system/pub/recognizer.h,v
retrieving revision 1.5
diff -u -w -1 -0 -r1.5 recognizer.h
--- pub/recognizer.h 7 Feb 2006 19:21:27 -0000 1.5
+++ pub/recognizer.h 20 Feb 2009 23:58:12 -0000
@@ -44,23 +44,39 @@
STDMETHOD(SeekDone) (THIS_ HX_RESULT status);
STDMETHOD(ReadDone) (THIS_ HX_RESULT status, IHXBuffer *pBuffer);
STDMETHOD(WriteDone) (THIS_ HX_RESULT status);
STDMETHOD(CloseDone) (THIS_ HX_RESULT status);
private:
STDMETHOD(GetMimeType) (THIS_ const char* /* IN */pFileName,
IHXBuffer* /* IN */ pBuffer,
REF(IHXBuffer*) /* OUT*/ pMimeType);
- HXBOOL IsSDPFile(IHXBuffer* pBuffer);
+ HXBOOL IsSDPFile(UCHAR* pData, UINT32 ulSize);
+ HXBOOL IsRMFile(UCHAR* pData, UINT32 ulSize);
+ HXBOOL IsMP3File(UCHAR* pData, UINT32 ulSize);
+ HXBOOL IsISOMediaFile(UCHAR* pData, UINT32 ulSize, const char* pBrand);
+ HXBOOL IsMP4File(UCHAR* pData, UINT32 ulSize);
+ HXBOOL Is3GPPFile(UCHAR* pData, UINT32 ulSize);
+ HXBOOL IsWMFile(UCHAR* pData, UINT32 ulSize);
+ HXBOOL IsAVIFile(UCHAR* pData, UINT32 ulSize);
void DoFileRecognize(void);
+ UINT32 LoadMP3Header(UCHAR* pData);
+ HXBOOL IsValidMP3Header(UINT32 ulHdr);
+ INT32 getFrameSync(UINT32 ulHdr) { return
(int)((ulHdr>>21) & 2047); };
+ INT32 getVersionIndex(UINT32 ulHdr) { return
(int)((ulHdr>>19) & 3); };
+ INT32 getLayerIndex(UINT32 ulHdr) { return
(int)((ulHdr>>17) & 3); };
+ INT32 getBitrateIndex(UINT32 ulHdr) { return
(int)((ulHdr>>12) & 15); };
+ INT32 getFrequencyIndex(UINT32 ulHdr) { return
(int)((ulHdr>>10) & 3); };
+ INT32 getEmphasisIndex(UINT32 ulHdr) { return (int)(ulHdr & 3); };
+
private:
LONG32 m_lRefCount;
IUnknown* m_pContext;
IHXFileRecognizerResponse* m_pResponse;
IHXFileObject* m_pFile;
IHXBuffer* m_pBuffer;
HXBOOL m_bGetMimeTypeDone;
};
#endif // _RECOGNIZER_H_