Closed inobelar closed 2 months ago
Odd. I checked for race conditions but it seems that my plugin's buffer processing functions are always called from the same thread. Furthermore, I think there is a lock present at a higher level - GST_OBJECT_LOCK - so it should not have been an issue.
I checked for payload sizes and saw that encryption payloads sizes are multiples of 16 but decryption payload size is 1 byte lesser than a multiple of 16 when the error occurs. This occurs at ciphertext level. Only variable that affects ciphertext length is emulation prevention bytes and there was none of them in the ciphertext in my tests. So I do not suspect them.
Perhaps there is some off-by-one error somewhere and setting thread count to a higher value enables it.
It is odd why it only happens in this condition so far though.
Hi there, can you check "pr4-test-fix" branch? I applied a test fix for the issue. It should help with pr5 as well.
The issue was when ciphertext ends with a null byte, it was not parsed / it was ignored by the decryptor's nal parser. H.264 RFC section B.1.1 says this is "trailing_zero_8bits" at the end of NAL unit and they are discarded.
I added a 0x80 byte to the end of each ciphertext and removed it at the decryptor before decrypting to fix it.
Hi! I tested your branch and I can say that the fixes solve the problems in both issues (https://github.com/ReFormationPro/Gstreamer-H264-Encryption/issues/4, https://github.com/ReFormationPro/Gstreamer-H264-Encryption/issues/5)!
Can you also add accompanying comments in new branch, with useful "why this and not otherwise" information? For example:
/* The ciphertext may end with a null byte, so we always add a non-null
* marker-byte at the end. (0x80 value is arbitrary)
*
* Without it (when ciphertext ends with a null byte), it was not
* parsed / it was ignored by the decryptor's nal parser.
* H.264 RFC section B.1.1 says this is "trailing_zero_8bits" at the end
* of NAL unit and they are discarded.
*/
#define CIPHERTEXT_END_MARKER 0x80
// Enlarge buffer by 1 byte, its needed to store extra 'non-null' marker-byte
// NOTE: Is it correct explanation ? :)
if (G_UNLIKELY(!gst_buffer_resize_range(outbuf, 0, -1, 0, dest_offset))) {
-1
as length
), after looking into gst_buffer_resize_range signature:
gboolean gst_buffer_resize_range (GstBuffer * buffer, guint idx, gint length, gssize offset, gssize size)
input_size + 40 + AES_BLOCKLEN + 30
, now input_size + 40 + AES_BLOCKLEN + 210
Can you add clarification comments about 40
and 210
constants ? (and why 30
is not enough, and must used 210
, or explain heuristics)Also, as I can see, similar to CIPHERTEXT_END_MARKER
constant 0x80
used in the next cases:
CIPHERTEXT_END_MARKER
too? (if so - why?) Or its different arbitrary value (and may have any other value)?Additionally (its not related to commits in the new branch), I found unclear the next section: https://github.com/ReFormationPro/Gstreamer-H264-Encryption/blob/697e19eaa0b438bf8c664b2675a7013146ae4d47/gst-h264-encryption/src/h264_encryption_plugin.h#L52-L56
GST_H264_ENCRYPT_IV_SEI_UUID
length (16-bytes) requires to be 16-bytes due to SEI format, or 16-bytes due to AES_BLOCKLEN
? As I can see, its always 16-bytes length, no matter what AES_KEYLEN
(128/192/256) used: https://github.com/ReFormationPro/Gstreamer-H264-Encryption/blob/697e19eaa0b438bf8c664b2675a7013146ae4d47/gst-h264-encryption/src/ciphers/aes.h#L32
Or its 16-bytes-length due to currently choosed AES 128-bit AES_KEYLEN
(16-bytes):
https://github.com/ReFormationPro/Gstreamer-H264-Encryption/blob/697e19eaa0b438bf8c664b2675a7013146ae4d47/gst-h264-encryption/src/ciphers/aes.h#L34-L44"\x06\x05\x20"
3-bytes prefix is unclear here. what first 2 bytes 0x06
0x05
means? They are arbitrary, or its related to SEI format? Why we need to change only third byte 0x20
, and why its 0x20
(decimal: 32), not 16 ?Thanks for your amazing work & quick responses!
Thank you for your kind comments.
I can add comments, sure. I am looking at other projects right now and as there is not a bug here, I am giving a break to this.
Closing this issue as pr4-test-fix is merged to the main. I also added some comments based on your remarks. Thanks for your help!
Greetings! I'm trying to use 'slightly tuned for performance'
x264enc
, and noticed, that settingthread
property value greater then1
, causes (after ~3-15 seconds) crash ofh264decrypt
. Single-threadedx264enc
works well for a long time.Here is my gstreamer pipeline:
GST_PLUGIN_PATH
- used to specify search path for locally-builth264encrypt
&h264decrypt
GST_DEBUG=3
- used to show detailed error messagesDISPLAY=:0.0
- I'm set it, since launch pipeline via ssh, its dont matterEnvironment:
STDOUT LOG (Execution ended after 0:00:15 seconds)
Setting pipeline to PAUSED ... 0:00:00.112587847 2439 0x7bae10000b70 FIXME default gstutils.c:4088:gst_element_decorate_stream_id_internal: