-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds a test for checking that subsample data is correctly parsed #45
Conversation
test/regress.c
Outdated
if (pkt_encryption == NESTEGG_PACKET_HAS_SIGNAL_BYTE_PARTITIONED) { | ||
nestegg_packet_offsets(pkt, &pkt_partition_offsets, &pkt_num_offsets); | ||
sha1_init(&s); | ||
sha1_write(&s, (char const *) pkt_partition_offsets, pkt_num_offsets * 4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way I should be doing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use sizeof(*pkg_partition_offsets)
rather than 4
This is probably the wrong way to do it for big-endian compatibility though, because unlike the other places we're doing a sha1_write
(at least, from a quick scan), this data has been converted to native endian during parsing, whereas the others are just hashing a byte buffer read directly from the file.
So maybe it's better to loop over the pkt_partition_offsets
and printf them all individually, along with the pkt_num_offsets
.
@@ -18,6 +18,7 @@ do_test bug1200148.webm -l | |||
do_test dancer1.webm | |||
do_test dancer1rb.webm | |||
do_test seek_encrypted.webm | |||
do_test subsample_encrypted.webm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be added twice? Once here, and once below, with -r set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please. -r tests the resumable parsing hack required for MSE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, should this have -l set?
8f36438
to
9cd4080
Compare
Rebased and merged as e9d6574, thanks! |
No description provided.