Re: [libav-devel] [PATCH] matroskaenc: fix writing ProjectionPrivate element on Cubemap projections

[Vittorio Giovara]

On Fri, Apr 7, 2017 at 11:54 AM, Hendrik Leppkes  wrote:
> On Fri, Apr 7, 2017 at 11:32 AM, Vittorio Giovara
>  wrote:
>> On Thu, Apr 6, 2017 at 10:45 PM, James Almer  wrote:
>>> Calling put_ebml_binary() with sizeof(private) as argument is currently
>>> only valid for Equirectangular projections.
>>>
>>> Signed-off-by: James Almer 
>>> ---
>>> Please, be more careful when making cosmetic changes to make sure they
>>> don't also change how the code works.
>>> Not only this was writing invalid Cubemap files, but also dumping
>>> uninitialized data from stack in them.
>>
>> Hello James,
>> unfortunately this mistake was present in the original patch
>> (https://patches.libav.org/patch/63023/) and the cosmetic change you
>> mention was only to break the line past the 80th column.
>> Thanks for the fix anyway, probably Luca's approach is more future proof 
>> though.
>
> The issue wasn't present in the original patch because it used a
> separate (properly sized) private buffer, instead of using a shared
> one for all cases.

ok my bad then
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] matroskaenc: fix writing ProjectionPrivate element on Cubemap projections

[Hendrik Leppkes]

On Fri, Apr 7, 2017 at 11:32 AM, Vittorio Giovara
 wrote:
> On Thu, Apr 6, 2017 at 10:45 PM, James Almer  wrote:
>> Calling put_ebml_binary() with sizeof(private) as argument is currently
>> only valid for Equirectangular projections.
>>
>> Signed-off-by: James Almer 
>> ---
>> Please, be more careful when making cosmetic changes to make sure they
>> don't also change how the code works.
>> Not only this was writing invalid Cubemap files, but also dumping
>> uninitialized data from stack in them.
>
> Hello James,
> unfortunately this mistake was present in the original patch
> (https://patches.libav.org/patch/63023/) and the cosmetic change you
> mention was only to break the line past the 80th column.
> Thanks for the fix anyway, probably Luca's approach is more future proof 
> though.

The issue wasn't present in the original patch because it used a
separate (properly sized) private buffer, instead of using a shared
one for all cases.

- Hendrik
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] matroskaenc: fix writing ProjectionPrivate element on Cubemap projections

[Vittorio Giovara]

On Thu, Apr 6, 2017 at 10:45 PM, James Almer  wrote:
> Calling put_ebml_binary() with sizeof(private) as argument is currently
> only valid for Equirectangular projections.
>
> Signed-off-by: James Almer 
> ---
> Please, be more careful when making cosmetic changes to make sure they
> don't also change how the code works.
> Not only this was writing invalid Cubemap files, but also dumping
> uninitialized data from stack in them.

Hello James,
unfortunately this mistake was present in the original patch
(https://patches.libav.org/patch/63023/) and the cosmetic change you
mention was only to break the line past the 80th column.
Thanks for the fix anyway, probably Luca's approach is more future proof though.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] matroskaenc: fix writing ProjectionPrivate element on Cubemap projections

[Luca Barbato]

On 06/04/2017 22:45, James Almer wrote:
> Calling put_ebml_binary() with sizeof(private) as argument is currently
> only valid for Equirectangular projections.
> 
> Signed-off-by: James Almer 
> ---
> Please, be more careful when making cosmetic changes to make sure they
> don't also change how the code works.
> Not only this was writing invalid Cubemap files, but also dumping
> uninitialized data from stack in them.
> 
>  libavformat/matroskaenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 34d983324..f6dade751 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -696,7 +696,7 @@ static int mkv_write_video_projection(AVFormatContext *s, 
> AVIOContext *pb,
>  avio_wb32(, 0); // layout
>  avio_wb32(, spherical->padding);
>  put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE,
> -private, sizeof(private));
> +private, 12);
>  break;
>  default:
>  av_log(s, AV_LOG_WARNING, "Unknown projection type\n");
> 

Probably would be safer to use avio_tell() in both cases?

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH] matroskaenc: fix writing ProjectionPrivate element on Cubemap projections

[James Almer]

Calling put_ebml_binary() with sizeof(private) as argument is currently
only valid for Equirectangular projections.

Signed-off-by: James Almer 
---
Please, be more careful when making cosmetic changes to make sure they
don't also change how the code works.
Not only this was writing invalid Cubemap files, but also dumping
uninitialized data from stack in them.

 libavformat/matroskaenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 34d983324..f6dade751 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -696,7 +696,7 @@ static int mkv_write_video_projection(AVFormatContext *s, 
AVIOContext *pb,
 avio_wb32(, 0); // layout
 avio_wb32(, spherical->padding);
 put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE,
-private, sizeof(private));
+private, 12);
 break;
 default:
 av_log(s, AV_LOG_WARNING, "Unknown projection type\n");
-- 
2.12.1

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel