Reimar D?ffinger wrote:
> Hello,
> On Thu, Jan 11, 2007 at 11:41:43AM +0100, Baptiste Coudurier wrote:
>> Reimar D?ffinger wrote:
> [...]
>>> 2) Seeking does not work with encrypted files
>> An implementation using MXF Index would be nice.
>
> The current seeking function IMO still should be kept and fixed.
> I was thinking of making mxf_read_sync work more like the mpeg header
> search stuff, i.e. just search for the next occurence of 0x06,0x0E,0x2B,0x34
> and then returning the full 16 bytes from that position or so.
> The search function could then first look for a mxf_essence_element_key
> and then get the next either mxf_essence_element_key or
> mxf_encrypted_triplet_key, whichever comes first. I think that should
> work for both cases.
Sounds good
>>> 3) SSL configure stuff is needed - or alternatively a standalone AES
>>> implementation might be interesting as well...
>>> 4) Code needs quite a bit of general cleanup, it seems much too
>>> complicated to me for what it does
>> It seems pretty clear to me. What looks like complicated ? MXF is
>> complicated, take a look at mxflib for example. Also code is prepared to
>> support more complicated files (think about OP3C, OP2B, ...)
>
> Exactly the function you complained about being too complicated.
Ok
>>> @@ -173,6 +174,7 @@
>>> /* partial keys to match */
>>> static const uint8_t mxf_header_partition_pack_key[] = { 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02 };
>>> static const uint8_t mxf_essence_element_key[] = { 0x06,0x0e,0x2b,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01 };
>>> +static const uint8_t mxf_encrypted_triplet_key[] = { 0x06,0x0e,0x2b,0x34,0x02,0x04,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x7e,0x01,0x00 };
>>>
>> Key is not partial.
>
> ?? oh, you mean because of the comment? Well, should I add a "/* full
> key to match */" or how?
Yes
>>> #define IS_KLV_KEY(x, y) (!memcmp(x, y, sizeof(y)))
>>>
>>> @@ -181,20 +183,8 @@
>>>
>>> static int64_t klv_decode_ber_length(ByteIOContext *pb)
>>> {
>>> - int64_t size = 0;
>>> - uint8_t length = get_byte(pb);
>>> - int type = length >> 7;
>>> -
>>> - if (type) { /* long form */
>>> - int bytes_num = length & 0x7f;
>>> - /* SMPTE 379M 5.3.4 guarantee that bytes_num must not exceed 8 bytes */
>>> - if (bytes_num > 8)
>>> - return -1;
>>> - while (bytes_num--)
>>> - size = size << 8 | get_byte(pb);
>>> - } else {
>>> - size = length & 0x7f;
>>> - }
>>> + uint64_t size;
>>> + GET_BER(size, get_byte(pb), return -1;);
>>> return size;
>>> }
>> IMHO GET_BER is unneeded since it is only used in mxf.c, also return
>> -1;); looks quite ugly.
>
> Huh? Why "unneeded"? The problem is that klv_decode_ber_length can't be
> used on memory, how else to solve this except making two different
> functions? Though this change might be unneeded. Anyway, my GET_BER is a
> bit simplified compared to this function, maybe you can say if you're
> interested in me porting these simplifications (if you consider them
> such).
Yes please do.
>>>
>>> [...]
>>>
>> Is it needed to check for size every 2 lines ?
>
> If esp. we decode directly there probably is no reason for this function
> to operate on memory but instead could read directly from file. In this
> case all the checks could be removed. As I said, the code needs some
> serious cleanup, esp. this part.
I feel like more comfortable with reading from the file directly.
>>> - if (IS_KLV_KEY(klv.key, mxf_essence_element_key)) {
>>> - int index = mxf_get_stream_index(s, &klv);
>>> + if (IS_KLV_KEY(klv.key, mxf_essence_element_key) ||
>>> + IS_KLV_KEY(klv.key, mxf_encrypted_triplet_key)) {
>>> + int index;
>>> + av_get_packet(&s->pb, pkt, klv.length);
>>> + if (IS_KLV_KEY(klv.key, mxf_encrypted_triplet_key) &&
>>> + get_enc_src_klv(s, pkt, &klv) < 0)
>>> + av_log(s, AV_LOG_ERROR, "invalid encoded triplet\n");
>>> + index = mxf_get_stream_index(s, &klv);
>>> if (index < 0) {
>>> av_log(s, AV_LOG_ERROR, "error getting stream index\n");
>>> - url_fskip(&s->pb, klv.length);
>>> + av_free_packet(pkt);
>>> return -1;
>>> }
>>> /* check for 8 channels AES3 element */
>>> if (klv.key[12] == 0x06 && klv.key[13] == 0x01 && klv.key[14] == 0x10) {
>>> if (mxf_get_d10_aes3_packet(&s->pb, s->streams[index], pkt, klv.length) < 0) {
>>> av_log(s, AV_LOG_ERROR, "error reading D-10 aes3 frame\n");
>>> + av_free_packet(pkt);
>>> return -1;
>>> }
>>> - } else
>>> - av_get_packet(&s->pb, pkt, klv.length);
>>> + }
>>> pkt->stream_index = index;
>>> return 0;
>>> } else
>> Freeing packet is overkill, I have some files with broken track number,
>> that's why code is that way. Useless allocating/freeing should be
>> avoided where it can.
>
> I was not aware that not freeing packets is acceptable and handled
> correctly...
Ok. Somehow I think that checking for encrypted key before, and calling
mxf_decrypt_triplet directly would be cleaner. At this point maybe using
function pointers like in read_header, and getting packet. There will be
3 ways of retrieving data now. What do you think ?
>>>
>>> [...]
>>>
>> That key is Encrypted essence container label, also that key can be used
>> with JPEG 2000 like my files are.
>
> So where to get the codec id from??
>
Picture essence coding ul in descriptor, if present. If not then I need
to dig further in specs.
--
Baptiste COUDURIER GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A. http://www.smartjog.com
Key fingerprint 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312
RetroSearch is an open source project built by @garambo | Open a GitHub Issue
Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo
HTML:
3.2
| Encoding:
UTF-8
| Version:
0.7.4